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

[BUG] [Code Style] HttpStatus & Headers should not be imported from org.apache.hc.core5.http #3581

Open
3 tasks
peternied opened this issue Oct 20, 2023 · 1 comment
Labels
bug Something isn't working CCI College Contributor Initiative good first issue These are recommended starting points for newcomers looking to make their first contributions. triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.

Comments

@peternied
Copy link
Member

What is the bug?

In this codebase on main, we are using a newer version of the apache http client; however, in our currently supported major version, 2.x we don't use this version of the http client and cannot due to dependency conflicts. Any changes that reference org.apache.hc.core5.http.HttpStatus or org.apache.hc.core5.http.Header cannot be backported without manual conflict resolution.

What is the expected behavior?

Our code hygiene workflows should catch usages of these namespace and prevent them from being checked into the codebase. It would be a bonus to fixing the issue by switching to org.apache.http.HttpStatus & org.apache.http.Header which will work in both the 2.x and mainline branches or provide the specific recommendation.

Exit Criteria

  • Add a new checkstyle or spotless rule that prevents the use of the org.apache.hc.core5.http package for HttpStatus and Header classes.
  • Fix all existing references to use org.apache.http.HttpStatus & org.apache.http.Header
  • [Optional] Update spotlessApply so that it can switch the references to the preferred replacement OR have the error message indicate how to resolve the issue by suggesting the correct replacement class.
@peternied peternied added bug Something isn't working good first issue These are recommended starting points for newcomers looking to make their first contributions. untriaged Require the attention of the repository maintainers and may need to be prioritized CCI College Contributor Initiative labels Oct 20, 2023
@cwperks
Copy link
Member

cwperks commented Oct 20, 2023

@peternied Should org.apache.http.HttpStatus be used in the integrationTest package to make backporting to 2.x easier? The main branch has already upgraded to org.apache.HttpComponents v5 and v4 is used in 2.x. main still has a dependency on v4 because its needed by OpenSAML. See details in #2932.

The library was updated in #2166

Edit: This would be useful in the integrationTest package to prevent issues with backporting, but as soon as there is a version of OpenSAML compatible with v5 of the library then I think main will be updated to remove the dependencies on v4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CCI College Contributor Initiative good first issue These are recommended starting points for newcomers looking to make their first contributions. triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.
Projects
None yet
Development

No branches or pull requests

2 participants