eclipse-lyo#636 Add option to not add providers by default #637
Conversation
|
I made a proposal commit and removed the deprecated dependency. Furthermore, I have refactored the OslcClient to avoid duplicate code and added some unit-tests accordingly. Hth |
| @@ -9,7 +9,7 @@ | |||
| ### Deprecated | |||
|
|
|||
| ### Removed | |||
|
|
|||
| - Dependency to deprecated oslc4j-json4j-provider | |||
There was a problem hiding this comment.
- Dependency to deprecated `oslc4j-json4j-provider` from Lyo Client
There was a problem hiding this comment.
@berezovskyi is good to clarify/update before merging.
| @@ -9,7 +9,7 @@ | |||
| ### Deprecated | |||
|
|
|||
| ### Removed | |||
|
|
|||
| - Dependency to deprecated oslc4j-json4j-provider | |||
There was a problem hiding this comment.
@berezovskyi is good to clarify/update before merging.
| * which is available at http://www.eclipse.org/org/documents/edl-v10.php. | ||
| * | ||
| * Copyright (c) 2020 Contributors to the Eclipse Foundation See the NOTICE file(s) distributed with this work for | ||
| * additional information regarding copyright ownership. This program and the accompanying materials are made available |
There was a problem hiding this comment.
This seems to be an un-intended change. Please revert.
| } | ||
|
|
||
| public Response getResource (String url, Map<String, String> requestHeaders, String defaultMediaType, | ||
| private final String version; |
There was a problem hiding this comment.
The whole file has been changed.
Main reason is due to white-space changes. But other formatting changes are also introduced.
Please revert such changes unless they are intentional.
|
|
||
| public Response getResource(String url, Map<String, String> requestHeaders, String defaultMediaType, | ||
| String configurationContext, boolean handleRedirects) { | ||
| return doRequest(HttpMethod.GET, url, null, configurationContext, null, defaultMediaType, OSLCConstants.CT_RDF, |
There was a problem hiding this comment.
This is a significant refactoring, and looks promising. But I can't easily review it. Too many other (formatting) changes in the way.
If you are confortable with that @berezovskyi, I am fine.
|
The original change to control the Providers looks good. But noticed other refactoring that I could not review. but @jhemm2 ! Please fix the unintended changes. I am afraid we will endup with many back-forth changes due to diffferent IDE settings. |
|
Yes, I am fine with the changes. Reformatting the code touched by the PR is intentional and welcomed/required. With regards to the settings, there is sn .editorconfig file in the repo that everyone (cough) is expected to apply when reformatting. But I have two further ideas on my bucket list:
|
|
Two more bits:
|
|
@berezovskyi editorconfig seems to be in place, but not sure if it makes a difference. |
|
can @jhemm2 commit to master, or should we do it? |
|
@Jad-el-khoury not unless @jhemm2 becomes a committer ;) |
Description
Adds another Constructor the OslcClient to allow custom registration of providers for the Client. This is due to registration of deprecated json providers by default
Checklist
Issues
Closes #636 ;
(use exactly this syntax to link to issues, one issue per statement only!)