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

Feature/optimize rules specifications artifact #209

Merged
merged 7 commits into from
Oct 27, 2023

Conversation

jycr
Copy link
Contributor

@jycr jycr commented Jul 10, 2023

@jycr jycr force-pushed the feature/optimize-rules-specifications-artifact branch from 95c2d26 to 1fd4d7a Compare July 11, 2023 09:03
@jycr jycr requested a review from dedece35 July 12, 2023 09:41
Copy link
Member

@dedece35 dedece35 left a comment

Choose a reason for hiding this comment

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

maybe having a meeting point on this PR to understand all modifications

@dedece35
Copy link
Member

Hi @jycr
could you correct biuld / test errors please before all ?

@jycr jycr force-pushed the feature/optimize-rules-specifications-artifact branch from 1fd4d7a to fc64482 Compare July 16, 2023 13:32
@jycr
Copy link
Contributor Author

jycr commented Jul 16, 2023

Hi @jycr could you correct biuld / test errors please before all ?

@dedece35 : I think this issue is on SonarCloud side. Isn't it?

Caused by: java.lang.IllegalStateException: Status returned by url [https://sonarcloud.io/batch/index] is not valid: [503]

@dedece35
Copy link
Member

Hi @jycr,
sorry again :( but please correct the conflicts :(

@jycr jycr force-pushed the feature/optimize-rules-specifications-artifact branch 2 times, most recently from 12553a5 to ee6a455 Compare July 22, 2023 00:07
@jycr jycr requested a review from dedece35 July 22, 2023 00:08
@dedece35 dedece35 added dependencies Pull requests that update a dependency file 🏗️ refactoring refactoring for best practices 🔥 in progress 🔥 labels Jul 26, 2023
@jycr jycr force-pushed the feature/optimize-rules-specifications-artifact branch 3 times, most recently from b2c3a5d to 80264fc Compare August 4, 2023 12:37
@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

warning The version of Java (11.0.20) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

@github-actions
Copy link

github-actions bot commented Sep 8, 2023

This PR has been automatically marked as stale because it has no activity for 30 days.
Please add a comment if you want to keep the issue open. Thank you for your contributions!

pom.xml Outdated Show resolved Hide resolved
pom.xml Show resolved Hide resolved
utarwyn
utarwyn previously approved these changes Sep 27, 2023
Copy link
Member

@utarwyn utarwyn left a comment

Choose a reason for hiding this comment

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

Comparing the jar result with the version before this PR, I get the same result.
So, everything looks good to me 💯 Nice work!

@utarwyn utarwyn requested a review from dedece35 September 27, 2023 20:08
@dedece35
Copy link
Member

dedece35 commented Oct 6, 2023

And I don't know if there is a dedicated CHANGELOG file for the rules specifications artifact?

for me, no the actual CHANGELOG.md is for all modifications of the current repository.
Currently, there are modifiations for java plugin sources and ecocode-rules-specifiacations.
Later, once java plugin source will be on new repository, no problem to continue to fill actual CHANGELOG onely for rules specifications : like that we can see old modifications of this repo.
@utarwyn Not agree ?

dedece35
dedece35 previously approved these changes Oct 6, 2023
@utarwyn
Copy link
Member

utarwyn commented Oct 6, 2023

@dedece35
for me, no the actual CHANGELOG.md is for all modifications of the current repository. Currently, there are modifiations for java plugin sources and ecocode-rules-specifiacations. Later, once java plugin source will be on new repository, no problem to continue to fill actual CHANGELOG onely for rules specifications : like that we can see old modifications of this repo.

I ask this because today the versions are completely different between the Java plugin (1.4.0) and the rules specification artifact (0.0.3). So to track changes in a single changelog it might be more difficult? The idea of having two dedicated changelog files can solve this problem. And we can move the file concerning the Java plugin during the separation

Just a thought like that, it's as you wish 😄

@jycr
Copy link
Contributor Author

jycr commented Oct 22, 2023

@dedece35
for me, no the actual CHANGELOG.md is for all modifications of the current repository. Currently, there are modifiations for java plugin sources and ecocode-rules-specifiacations. Later, once java plugin source will be on new repository, no problem to continue to fill actual CHANGELOG onely for rules specifications : like that we can see old modifications of this repo.

I ask this because today the versions are completely different between the Java plugin (1.4.0) and the rules specification artifact (0.0.3). So to track changes in a single changelog it might be more difficult? The idea of having two dedicated changelog files can solve this problem. And we can move the file concerning the Java plugin during the separation

Just a thought like that, it's as you wish 😄

Indeed, I publish manually rule-specification under 0.x version.
When all automatic process (Github Workflow) will be OK, we can switch to 2.x version for rules?

@jycr jycr dismissed stale reviews from dedece35 and utarwyn via e2114d2 October 22, 2023 15:53
@jycr jycr force-pushed the feature/optimize-rules-specifications-artifact branch from 36bef1f to e2114d2 Compare October 22, 2023 15:53
@jycr jycr force-pushed the feature/optimize-rules-specifications-artifact branch from e2114d2 to 45ab514 Compare October 22, 2023 16:06
@dedece35
Copy link
Member

dedece35 commented Oct 22, 2023

@utarwyn we can indeed create 2 changelog.md files, no problem for me. Maybe it will be clearer for all.

@jycr, for me, before to upgrade version number, I think we have to migrate Java PRs and issues to new repository.
In this repository, it remains only Java language plugin (PHP an python plugins have already been moved to their repository during last summer).
In fact, for java language, there is a lot of PRs and it will be time expensive to migrate one by one to new repository. That's why I reviewed a maximum of them and try to contact commiter to make modifications.
Regarding our internal process, if we have no feedback from commiter within 1 month, I will create a new PR based on commiter PR and will close this last one. Thus, when I will create this kind PR, I think I will initiate the new Java language repository and create each PR into it.

On the other way, I think changing version from 0.0.X to 2.Y doesn't respect best practice on version management. I think we have to go from 0.0.X to 1.0.0 for ecocode rule specifications.

What do you think about it, @utarwyn and @jycr ?

Finally, I try this PR locally with a build and deploy on local docker. When SonarQube starts, I have an error :(
"java.lang.NoClassDefFoundError: org/sonarsource/analyzer/commons/RuleMetadataLoader"

@jycr did you try your PR locally ?

@dedece35 dedece35 added the 👀 👀 review done 👀 👀 review done - waiting for changes label Oct 22, 2023
@jycr
Copy link
Contributor Author

jycr commented Oct 22, 2023

@jycr did you try your PR locally ?

Yes.
But with a fresh LTS install with only java-plugin, I can reproduce the issue.
I will check that!
Thanx

@utarwyn
Copy link
Member

utarwyn commented Oct 24, 2023

@dedece35
I propose to wait for the new Java repository, and after we will be able to create dedicated changelogs in each repository, so the problem will be solved. I agree that the first stable version of the rules specification artifact must be 1.0.0 and not a 2.x.x. We have to move forward because we've been dragging this repo separation out for a long time. I'm in favor of transferring and re-creating needed PRs in the dedicated repository.

BUT I think that the first action should be to integrate this PR in order to process other PRs concerning the rules specification artifact. Some big enhancements in plugins are waiting on this one 😉

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jycr jycr requested review from utarwyn and dedece35 October 27, 2023 15:58
@jycr jycr merged commit 47ae217 into main Oct 27, 2023
3 checks passed
@jycr jycr deleted the feature/optimize-rules-specifications-artifact branch October 27, 2023 16:46
@jycr
Copy link
Contributor Author

jycr commented Oct 27, 2023

@utarwyn : https://repo1.maven.org/maven2/io/ecocode/ecocode-rules-specifications/0.0.5/
Enjoy ! ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏗️ refactoring refactoring for best practices dependencies Pull requests that update a dependency file 👀 👀 review done 👀 👀 review done - waiting for changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants