Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove and doc unnecessary overrides (now added by default) #2008

Merged
merged 4 commits into from
Jun 27, 2019

Conversation

ignasi35
Copy link
Contributor

related to #1922

@ignasi35 ignasi35 requested a review from a team as a code owner June 26, 2019 10:04
@ignasi35 ignasi35 requested a review from octonato June 26, 2019 10:04
@ignasi35 ignasi35 self-assigned this Jun 26, 2019
@ignasi35 ignasi35 added this to the Lagom 1.6.0-M3 milestone Jun 26, 2019
@ignasi35
Copy link
Contributor Author

Despite #1922 being backported to 1.5.x this PR only documents the Migration16.md. Users of 1.5.2+ may remove the setting but are not instructed to do it on the Migration15.md because we only made the value a default in 1.5.2.

@dwijnand
Copy link
Contributor

In #2007 I dropped the suggestion to remove the override of the cluster sharding strategy as I think it's noise that detracts from the more critical information in the guide (e.g. cluster shutdown).

So I suggest the same here (explicitly this time): if the configuration is just redundant but otherwise safe to have, I wouldn't include the detail on removing it.

But I'm interested if anyone disagrees (maybe we can find a compromise).

@ignasi35
Copy link
Contributor Author

SGTM. Let me try again ;-)

@ignasi35
Copy link
Contributor Author

Actually, I'll rebase on top of #2007 to simplify future merges (fewer conflicts)

@ignasi35 ignasi35 force-pushed the remove-protobuf-serializers-overrides branch from 24c1113 to e4dbd52 Compare June 26, 2019 10:29
@ignasi35
Copy link
Contributor Author

Build failed on stalled execution and artifact download

@ignasi35 ignasi35 force-pushed the remove-protobuf-serializers-overrides branch from e4dbd52 to b6d1fe3 Compare June 27, 2019 09:17
@dwijnand dwijnand merged commit 3de446e into lagom:master Jun 27, 2019
@TimMoore
Copy link
Contributor

@ignasi35 what do you think about removing the bindings from the Maven archetype only in 1.5.x? Less stuff to understand in a new Lagom 1.5 project, and less stuff to clean up when migrating.

They were removed from the giter8 templates in lagom/lagom-java.g8#66 and lagom/lagom-scala.g8#66.

@ignasi35
Copy link
Contributor Author

@ignasi35 what do you think about removing the bindings from the Maven archetype only in 1.5.x? Less stuff to understand in a new Lagom 1.5 project, and less stuff to clean up when migrating.

Sure! I'm currently rewriting the g8 seeds and was planning on reworking the maven archetype but I only had master in mind. Let's make this happen for 1.5.x too.

@ignasi35 ignasi35 deleted the remove-protobuf-serializers-overrides branch February 12, 2020 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants