Skip to content
This repository has been archived by the owner on Nov 23, 2021. It is now read-only.

issue #459 #600

Closed
wants to merge 1 commit into from
Closed

issue #459 #600

wants to merge 1 commit into from

Conversation

geoffroya
Copy link

  • Added bom-before-spring-boot/pom.xml and impact on samples (Work in progress - still some tests failed)
  • Modified oasp4j-samples to depends upon oasp4j and not to be child of it
  • Updated archetype
  • Updated DatabaseMigrator to use Flyway 4.2 flag ignoreMissingMigrations to keep calm when running Flyway only on specific scripts for tests

issue oasp#459
5e1a44e 
- Added bom-before-spring-boot/pom.xml and impact on samples (Work in progress - still some tests failed)
- Modified oasp4j-samples to depends upon oasp4j and not to be child of it
- Updated archetype
- Updated DatabaseMigrator to use Flyway 4.2 flag ignoreMissingMigrations to keep calm when running Flyway only on specific scripts for tests
@oasp-ci
Copy link
Collaborator

oasp-ci commented Aug 24, 2017

Can one of the admins verify this patch?

@geoffroya geoffroya mentioned this pull request Aug 24, 2017
Closed
@geoffroya
Copy link
Author

Hi all,

Any chance this PR can reviewed?

Best regards

Geoff.

@ssarmokadam
Copy link
Contributor

I have analyzed issue#459 and PR#600 for it. Issue#459 is for migrating flyway version from 3.x to 4.x (This migration is suggested as there are some issues in 3.x which causes unnecessary pain e.g flyway/flyway#253 ). PR#600 includes changes for this migration. But in my opinion, this changes will be incorporated while we will upgrade version of spring boot to 2.0.0. Spring boot 2.0.0 includes flyway version 4.2.0. I have reffered version here https://docs.spring.io/spring-boot/docs/current-SNAPSHOT/reference/htmlsingle/#appendix-dependency-versions .Also flyway/flyway#253 doesn't look like flyway issue , it is related to ending spaces which is more related to OS. @maybeec do you agree or you have any other view?

@maybeec
Copy link
Member

maybeec commented Oct 11, 2017

As far as of my latest knowledge, we want to maintain two versions of oasp4j in the next releases. One 2.x branch as legacy for running projects, which do not want to upgrade and we will have a 3.x releases with the newest versions including the spring boot 2 upgrade and other major refactorings.
I think the question here is more, whether we want to support the latest flyway version in oasp4j 2.x branch and I would claim we want. Even better is, that we already got a PR here. Thanks!

@geoffroya
Copy link
Author

Hi
The reason for us to upgrade is to manage Repeatable migration scripts (supported from 4.0) that could not be executed with the version shipped with Spring-boot 1.x.

We think that's quite an improvment for dev teams.

@geoffroya
Copy link
Author

Hi all,

What prevents merging of this PR?

Best regards

@hohwille
Copy link
Member

hohwille commented Nov 7, 2017

@geoffroya thanks for your PR. Good work! 👍 I am happy to get this proceeded. However some feedback from my side:

  • I do not see the real purpose of bom-before-spring-boot nor is its name self-explanatory and violates conventions. I always suggested that we should have a oasp4j-core-bom with only the oasp4j module artifact versions but without spring-boot and external dependencies. Then oasp4j-bom can derive from that and remain compatible. However, then one can go to a next spring-boot release without waiting for an oasp4jupdate. That said, I assume that your bom-before-spring-boot was introduced to change the flyway version that was introduced via spring-boot (version override). Therefore you do not need an additional POM/BOM. Just set flyway.version in properties section. The current approach is not very elegant. Do you agree or did I miss something? Please let us know if you want to update your PR accordingly or if you are most likely MIA and we should take over.
  • Setting ignoreMissingMigrations might need some revisit but I am fine with the rest of the changes.
  • FYI: 2.4.0 was already released - you should have branched 2.5.0. For your next PR you should read https://github.com/oasp-forge/oasp4j-wiki/wiki/oasp-code-contributions in advance. For now you do not need to worry. We can change the base branch when merging (but actually a feature PR shall never change the POM version).

@maybeec Let us discuss the compatibility and roadmap stuff in the next lead architects meeting on Thursday. In general I am for updating versions recently but this change is indeed somewhat critical. IMHO the bug in the old release made "real projects" to upgrade already... Should we raise a Yammer poll to ask community if someone would be affected by such change?

@maybeec
Copy link
Member

maybeec commented Nov 7, 2017

I already added it to the agenda of next architects meeting. Good idea to check the implications directly via a poll. To have an answer for further discussion, we should also have some idea about the upgrade efforts. @hohwille do you have already experiences with that?

@geoffroya
Copy link
Author

Thank you for the feedback

@hohwille, changing the flyway version directly in core project does the trick - only for core project. If another project depends on core, it will get FlyWay 3.2.1.

The before-spring-boot POM allows to override flyway - even for the core dependant projects.

@hohwille
Copy link
Member

@geoffroya thanks for your explanation. This seems to be a real problem with maven dependencyManagement. Meanwhile I added PR #623 that can be used as "workaround" to use oasp4j without being forced for all versions of the OSS dependencies. Still I am looking for a better solution.
For flyway upgrade we have #459 and #620.
The archetype is completely reworked with PR #629.
The restaurant is going to be deleted entirely as it is replaced with MTS.
Therefore it is most likely that we are not going to merge this PR anymore. We might cherry-pick some changes from it but as a whole it does not make sense anymore. Thanks for your work - I guess all its value will somehow make it into 3.0.0 but maybe via some detours.

@geoffroya
Copy link
Author

geoffroya commented Jan 31, 2018 via email

@hohwille
Copy link
Member

PR #623 has been merged meanwhile and reach the world with release 2.6.1 this month.
Also we do have flyway 4 since 2.6.0. Finally version 3.0.0 will come with flyway 5.
If that problem still persists please let me know. However, I could not reproduce this with 2.6.x.
Starting with 2.6.1 you will be able to import oasp4j-minimal-bom to your project together with the spring boot BOM of your choice and avoid version clashing.

If no further feedback is provided in the next weeks I am going to close this. Thanks for your feedback. I hope we now got everything right.

@geoffroya
Copy link
Author

geoffroya commented Jun 30, 2018 via email

@hohwille hohwille closed this Aug 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@geoffroya @oasp-ci @ssarmokadam @maybeec @hohwille