Skip to content

fix: align Action / View name convention#405

Merged
patriknw merged 1 commit intomainfrom
rgc/more-hacks-around-names
Sep 16, 2021
Merged

fix: align Action / View name convention#405
patriknw merged 1 commit intomainfrom
rgc/more-hacks-around-names

Conversation

@octonato
Copy link
Copy Markdown
Member

Refs: #238

Align the convention and add Impl if name contains Action|View (not only at the end)

@octonato
Copy link
Copy Markdown
Member Author

reverting to draft, I messed up with something here

@octonato octonato marked this pull request as draft September 16, 2021 09:57
@octonato octonato force-pushed the rgc/more-hacks-around-names branch from d53f90a to 5f33bbb Compare September 16, 2021 10:11
Copy link
Copy Markdown
Contributor

@patriknw patriknw left a comment

Choose a reason for hiding this comment

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

looking good, is it ready?

@octonato octonato force-pushed the rgc/more-hacks-around-names branch from 5f33bbb to 8d18593 Compare September 16, 2021 12:06
@octonato octonato marked this pull request as ready for review September 16, 2021 12:06
@octonato
Copy link
Copy Markdown
Member Author

Ready now, but surprisingly the registration order in in valueentity-customer-registry changed. 🤷

CustomerSummaryByNameView::new,
CustomerByEmailView::new,
CustomersResponseByNameView::new,
CustomerByEmailView::new,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

So, here the names didn't change, but the ordering changed. Note that the CustomerS and Customers (plural).

In any case, the ordering doesn't seen to be deterministic.

CustomerByEmailView
CustomerActionImpl
CustomerByNameView

Alphabetical? Per component type? File system ordering?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I created issue #407 for this.


import static org.junit.Assert.*;

public class CustomerEntityTest {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I compiled all project locally and found some files that were not checked in

@patriknw
Copy link
Copy Markdown
Contributor

To be strict, the commit comment should be fix! since it's a breaking change. Not sure how strict we are yet with versioning of such changes. I think @retgits has hinted that we should deliver fast without too much worry of breaking user's code. It's just some cases of Action/View names that will be changed.

@octonato octonato force-pushed the rgc/more-hacks-around-names branch from eb5ebe9 to 1cd6ee5 Compare September 16, 2021 13:45
@octonato
Copy link
Copy Markdown
Member Author

I changed the commit message and fixed the build, we had a failure in the codegen.
I will look into #407

Copy link
Copy Markdown
Contributor

@patriknw patriknw left a comment

Choose a reason for hiding this comment

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

LGTM

@patriknw patriknw merged commit 65b8523 into main Sep 16, 2021
@patriknw patriknw deleted the rgc/more-hacks-around-names branch September 16, 2021 14:20
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.

2 participants