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

527 added support for additional request headers in OslcQuery #553

Conversation

jhemm2
Copy link
Contributor

@jhemm2 jhemm2 commented Jun 13, 2024

Description

Added support for additional request headers when creating and OSLCQuery

Checklist

  • This PR adds an entry to the CHANGELOG. See https://keepachangelog.com/en/1.0.0/ for instructions. Minor edits are exempt.
  • This PR was tested on at least one Lyo OSLC server (e.g. manual workflow on Lyo Sample and OSLC RefImpl) or adds unit/integration tests.
  • This PR does NOT break the API

Issues

Closes #527

@jhemm2
Copy link
Contributor Author

jhemm2 commented Jun 13, 2024

I still can't get the .editorconfig to work, neither in eclipse nor VS Code - so sorry for the format changes. Maybe someone can do the re-formatting if needed.

@jhemm2 jhemm2 force-pushed the feature/527-support-additional-headers-in-oslcquery branch from 93c5059 to ed025c9 Compare June 17, 2024 05:56
@Jad-el-khoury
Copy link
Contributor

@jhemm2 The format changes are fine. It is actually an improvement where you are replacing our "tabs" with "white spaces".

The change looks fine to me otherwise.
Have you tested this on a Lyo server/client?

@berezovskyi! The SonarCloud Code Analysis failed. What is the implications for this?

@jhemm2 jhemm2 force-pushed the feature/527-support-additional-headers-in-oslcquery branch from ed025c9 to 717ea0d Compare June 20, 2024 15:19
@jhemm2
Copy link
Contributor Author

jhemm2 commented Jun 20, 2024

Hello @Jad-el-khoury
I have tested a query with the additional request headers on client side, which is working fine.
I have just added some tests - they may not be that useful but do some invocation of the overloaded constructors. Maybe this will already fulfill the required code coverage for the sonar scan.

@berezovskyi
Copy link
Contributor

@jhemm2 thanks for the PR and the tests!

@Jad-el-khoury the Sonar check is just FYI for the committers, not a blocking one. I think @jhemm2 added sufficient tests to merge.

Copy link
Contributor

@berezovskyi berezovskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last thing, could you please add an entry under https://github.com/eclipse/lyo/blob/master/CHANGELOG.md#added like

  • Support for additional request headers to OslcQuery.

Copy link

sonarcloud bot commented Jun 20, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 30%)

See analysis details on SonarCloud

@jhemm2 jhemm2 force-pushed the feature/527-support-additional-headers-in-oslcquery branch from 717ea0d to bbc789c Compare June 21, 2024 05:34
@berezovskyi berezovskyi merged commit 0ac6506 into eclipse-lyo:master Jun 21, 2024
6 checks passed
@berezovskyi
Copy link
Contributor

Thank you again for the contribution, @jhemm2! It will land in Lyo Snapshot repo in less than an hour and on Central when we release Lyo 6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support additional request headers in OslcQuery
3 participants