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.

Sign up for GitHub

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

Jump to bottom

refactor: Uses standard org.sonar.check.Rule annotation #16

Closed
wants to merge 4 commits into from
Closed

refactor: Uses standard org.sonar.check.Rule annotation #16

wants to merge 4 commits into from

Conversation

jycr
Copy link
Contributor

@jycr jycr commented Jul 9, 2023

fix: #15

@jycr jycr force-pushed the feature/ISSUE-3--Rename-rule-IDs branch 2 times, most recently from fb6029e to 6d9f3b6 Compare July 9, 2023 22:45
@jycr jycr force-pushed the feature/ISSUE-3--Rename-rule-IDs branch from 6d9f3b6 to afca5e7 Compare July 9, 2023 23:10
@dedece35
Copy link
Member

Hi @jycr,
before all, could you correct the build / test errors please ?

@jycr
Copy link
Contributor Author

jycr commented Jul 16, 2023 •
edited
Loading

Hi @jycr, before all, could you correct the build / test errors please ?

@dedece35 : I don't think this SonarCloud analysis is a problem of my PR, but a problem in repo configuration? Isn't it?

Error: Failed to execute goal org.sonarsource.scanner.maven:sonar-maven-plugin:3.9.1.2184:sonar (default-cli) on project ios-plugin: Project not found. Please check the 'sonar.projectKey' and 'sonar.organization' properties, the 'SONAR_TOKEN' environment variable, or contact the project administrator -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.sonarsource.scanner.maven:sonar-maven-plugin:3.9.1.2184:sonar (default-cli) on project ios-plugin: Project not found. Please check the 'sonar.projectKey' and 'sonar.organization' properties, the 'SONAR_TOKEN' environment variable, or contact the project administrator

@dedece35 dedece35 requested review from dedece35, zippy1978 and jduigoudev and removed request for dedece35 July 23, 2023 10:29
@dedece35 dedece35 added the 🏗️ refactoring refactoring for best practices label Jul 23, 2023
@github-actions
Copy link

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!

@github-actions github-actions bot added the stale Stale label Aug 23, 2023
zippy1978
zippy1978 requested changes Nov 7, 2023
View reviewed changes
Copy link
Contributor

@zippy1978 zippy1978 left a comment

Choose a reason for hiding this comment

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

See comments

commons-ios/src/main/java/io/ecocode/ios/checks/ReportIssueRecorder.java
Copy link
Contributor

Choose a reason for hiding this comment

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

We should keep the repository parameter in case we want to reuse the issue recorder to register rules for other iOS compliant languages (such as Objective-C)

swift-lang/src/main/java/io/ecocode/ios/swift/EcoCodeSwiftRulesDefinition.java

private static final String NAME = Swift.REPOSITORY_NAME;
private static final String LANGUAGE = Swift.KEY;
private static final List<Class<?>> CHECK_CLASSES = List.of(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not using reflections to find existing rules automatically ?

@zippy1978 zippy1978 removed the stale Stale label Nov 7, 2023
@zippy1978 zippy1978 mentioned this pull request Nov 29, 2023
Merged
@zippy1978
Copy link
Contributor

Hi @dedece35 , @jycr,
As I need to merge this refact soon, I did address the comments of this one and issued a new PR here : #20

So you can close this one, and review the new one instead.

@dedece35
Copy link
Member

Hi @zippy1978,
as you wanted, PR closed.

@dedece35 dedece35 closed this Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zippy1978 zippy1978 zippy1978 requested changes

@jduigoudev jduigoudev Awaiting requested review from jduigoudev

Assignees

@jycr jycr

Labels
🏗️ refactoring refactoring for best practices
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactoring: use same mechanism to manage/load check rules
3 participants
@jycr @dedece35 @zippy1978