Skip to content

More Descriptive Input/Output class names#2487

Closed
ragboyjr wants to merge 1 commit into
api-platform:2.4from
ragboyjr:more-descriptive-input-class-names
Closed

More Descriptive Input/Output class names#2487
ragboyjr wants to merge 1 commit into
api-platform:2.4from
ragboyjr:more-descriptive-input-class-names

Conversation

@ragboyjr

@ragboyjr ragboyjr commented Feb 3, 2019

Copy link
Copy Markdown
Contributor
  • Updated the DocumentationNormalizer to give
    a more decriptive name for input/output classes
  • This looks very good when you name your DTO's
    like commands: Book:CreateBookRequest or
    Book:UpdateBookForm etc etc.

Signed-off-by: RJ Garcia rj@bighead.net

Q A
Bug fix? No
New feature? Ish
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

- Updated the DocumentationNormalizer to give
  a more decriptive name for input/output classes
- This looks very good when you name your DTO's
  like commands: `Book:CreateBookRequest` or
  `Book:UpdateBookForm` etc etc.

Signed-off-by: RJ Garcia <rj@bighead.net>

@dunglas dunglas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed it looks better, but it isn't conflict free: if you have two DTO named Foo in different namespaces, it will conflict, it wasn't the case before.

@ragboyjr

ragboyjr commented Feb 3, 2019

Copy link
Copy Markdown
Contributor Author

Yes but they are still prefixed by the main resource short name. So you’d have to use two same names input in different namespaces for the same main resource to see this conflict

@soyuka

soyuka commented Mar 6, 2019

Copy link
Copy Markdown
Member

In fact we should first do the missing part of input/output which is to have a list of all the classes (resources + i/o) supported. These will then be used to generate a hydra context. This will allow to detect conflicts before they actually are an issue.

There's a bit more work to do on this before we can change the name in the documentation.

@er1z

er1z commented Mar 19, 2019

Copy link
Copy Markdown
Contributor

This will allow to detect conflicts before they actually are an issue.

Conflicts occur right now if you rely on DTOs. So this MR will not break already broken things. :D

@er1z

er1z commented Mar 19, 2019

Copy link
Copy Markdown
Contributor

Anyway, keeping md5 is a weird option because upon swagger-codegen we have non-semantic response class name.

@dunglas

dunglas commented Oct 14, 2019

Copy link
Copy Markdown
Member

Can you rebase this one @ragboyjr? I would like to merge it. A future enhancement would be to make the name configurable.

@soyuka

soyuka commented Oct 14, 2019

Copy link
Copy Markdown
Member

and target 2.5 :p

@ragboyjr

Copy link
Copy Markdown
Contributor Author

sounds good guys!

Will do rebase off of 2.5 :)

@MLDMoritz

MLDMoritz commented Oct 29, 2019

Copy link
Copy Markdown
Contributor

I've seen many changes between 2.5 and 2.4 - it seems the generation of the name has moved, so just rebasing won't be enough.

Name generation is done here now:

$prefix .= ':'.md5($inputOrOutputClass);

@MLDMoritz MLDMoritz mentioned this pull request Oct 29, 2019
@soyuka

soyuka commented Nov 1, 2019

Copy link
Copy Markdown
Member

superseeded by #3228

@soyuka soyuka closed this Nov 1, 2019
@AlexeyKosov

Copy link
Copy Markdown

Is there a chance it will be possible to customize prefixes for different DTO's?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants