-
-
Notifications
You must be signed in to change notification settings - Fork 478
WFCORE-7350: Elytron schema bump to 19 #6518
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
base: main
Are you sure you want to change the base?
Conversation
|
Hello, theashiot. I'm waiting for one of the admins to verify this patch with /ok-to-test in a comment. |
597e958 to
42820c4
Compare
|
/ok-to-test |
|
Core -> WildFly Preview Integration Build 14794 outcome was FAILURE using a merge of 42820c4 |
|
Core -> Full Integration Build 14671 outcome was FAILURE using a merge of 42820c4 |
|
Core -> Full Integration (with SecurityManager) Build 14991 outcome was FAILURE using a merge of 42820c4 |
42820c4 to
2c3a21b
Compare
|
There has been no activity on this PR for 45 days. It will be auto-closed after 90 days. |
2c3a21b to
82a81d5
Compare
|
There is very little value in the PR like this as this is not mergeable on it's own. You need a change to go along with the schema bump. |
|
Core -> Full Integration (with SecurityManager) Build 15116 outcome was FAILURE using a merge of 82a81d5 Failed tests |
|
Thanks, @rhusar for the feedback! Creating a separate PR for version bump was a conscious decision. Otherwise, if different people are introducing changes to Elytron, they will make the bump in their own PR and thus it will be a duplication of effort and . This particular Issue + PR was created in anticipation of https://issues.redhat.com/browse/ELY-2921. Also, we had a discussion about this topic here: #wildfly-elytron > Elytron schema bump to 19. Darran also mentioned about having a single place for coordinating elytron schema bumps in this PR: #6557 (review). Please LMK what you think! @darranl the test failure (org.jboss.as.test.clustering.cluster.web.remote.FineHotRodPersistenceWebFailoverTestCase failure) seem to be unrelated to the version bump, but i could be wrong. Can you please have a look? Thanks, |
OK, no problem. Though, introducing a new schema in a well-designed subsystem should be trivial and virtually only couple of LOC changed + XSD copy. Also, just to state the obvious, not all changes in the model result in change in the schema change. Also, a schema change in most cases require model bump as well. Also, this is only adding a particular stability level (community) while others may be developing a feature at a different stability, making this change unusable by them (and eventually require a rebase anyway).
OK, I guess it's useful for visibility, but it's quite common to share topic branches without an existing PR (which is why git is so great!)
OK, np, you clearly know what you are doing 👍🏼
This is actually ours, safely ingore that one; tracked as https://issues.redhat.com/browse/WFLY-21180 |
|
@rhusar +1 to the comment from @theashiot we have multiple developers who may work on this subsystem concurrently, the approach we use is one coordinates in chat that they will handle the schema bump and then all other changes will build upon that bump. In the past we had people undertaking the same work and then having to fix their branches depending on who's bump made it in first. |
82a81d5 to
344637d
Compare
Topic branches? If there are multiple developers working on branches for the same model and/or schema version, they would typically base their respective locally branches on some common branch, e.g. theashiot:WFCORE-7350. |
Yes, that's what I am calling a topic branch such as this When I submitted my original comments, the PR was in draft state and that's why I didn't bring it up since I assumed that the understanding is shared, that things that are not in the mergeable state should be drafts. |
344637d to
6b6034c
Compare
|
Core -> WildFly Preview Integration Build 14942 outcome was FAILURE using a merge of 6b6034c Failed tests |
|
@theashiot At this point please don't change the SHAs of either of these commits so other PRs can depend upon them |
Just to mention, git is smart enough to skip the same commit even if the SHA differs. Also, even then, people can easily rebase last commit on top of the branch with no breakage, e.g. |
https://issues.redhat.com/browse/WFCORE-7350