jakartaee / rest

Jakarta RESTful Web Services
Other
351 stars 114 forks source link

Regression in release-4.0 branch #1176

Closed NicoNes closed 8 months ago

NicoNes commented 10 months ago

This PR is my proposal to fix multiple regressions introduced by the initial commit (https://github.com/jakartaee/rest/commit/ae1f6ef7f4db9957079b8c383041a4537f33f35b) on branch "release-4-0" (see issue https://github.com/jakartaee/rest/issues/1165).

My fix is made of 2 commits:

@spericas in your initial commit, you have massively replace all Context by Entity in javadoc in many classes (I listed them below). I do not revert these changes but I wonder if all of them are really relevant. If not antoher issue and PR will have to be opened to fix it.

-- Nicolas

spericas commented 10 months ago

@NicoNes Thanks for looking into this. Generally looks good but let's drop changes that only update import orders, especially static ones. I believe the original 4.0 branch was created too early (before my large PR) and went stale.

spericas commented 10 months ago

@NicoNes Let's create another issue for the Context/Entity replacement problem you identified. You can assign it to me.

NicoNes commented 10 months ago

@NicoNes Thanks for looking into this. Generally looks good but let's drop changes that only update import orders, especially static ones. I believe the original 4.0 branch was created too early (before my large PR) and went stale.

@spericas Ok I will drop those changes.
I wasn't sure about what do to with those static import order changes because according to the GIT history, @mkarg commit (38772e3353f5f9159adfa008e4e25566df9ca6a5) about reorganizing the imports, moved these imports from down to top. So I believed your changes to move them back down again was not intentional.

spericas commented 10 months ago

@NicoNes Thanks for looking into this. Generally looks good but let's drop changes that only update import orders, especially static ones. I believe the original 4.0 branch was created too early (before my large PR) and went stale.

@spericas Ok I will drop those changes. I wasn't sure about what do to with those static import order changes because according to the GIT history, @mkarg commit (38772e3) about reorganizing the imports, moved these imports from down to top. So I believed your changes to move them back down again was not intentional.

Sounds like we may need some consensus here. In any case, it probably shouldn't be part of this PR (or any PR not dealing specifically with that).

spericas commented 9 months ago

@mkarg Could you review this one too?