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

Issue #14715: Enforced new naming convention in IT area - Part One #14725

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

MANISH-K-07
Copy link
Contributor

@MANISH-K-07 MANISH-K-07 commented Mar 27, 2024

Part of #14715

PR in response to #14715 (comment)

I got rid of suffixes like "....One/1" wherever possible and replaced with proper suffixes.
For example :

  • SuppressionXpathRegressionArrayTrailingCommaOne --> InputXpathArrayTrailingCommaLinear
  • SuppressionXpathRegressionArrayTrailingCommaTwo --> InputXpathArrayTrailingCommaMatrix

In commit :

            "AbbreviationAsWordInName",
            "AbstractClassName",
            "AnnotationLocation",
            "AnnotationOnSameLine",
            "AnnotationUseStyle",
            "AnonInnerLength",
            "ArrayTrailingComma",
            "ArrayTypeStyle",

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

ok to merge

@rnveach
Copy link
Member

rnveach commented Mar 27, 2024

I believe it was mentioned we should stick to 20 files max and a round number of checks. This PR has 47 files but I will still review it.

@MANISH-K-07
Copy link
Contributor Author

MANISH-K-07 commented Mar 28, 2024

I believe it was mentioned we should stick to 20 files max and a round number of checks. This PR has 47 files but I will still review it.

@rnveach , I will further limit to 20 files / 2-3 checks max. It would take atleast 50 PRs to close issue in this case

@rnveach
Copy link
Member

rnveach commented Mar 28, 2024

This was why I was originally saying for others to assist with this and not have you do it alone. You can always create more than one PR at a time for this. The best way to accomplish this since we have the suppression list is to do items sporadically. Like do the first 2, skip 6 or so, do the next 2, and so on. That is the best way to avoid git conflicts when one PR is merged.

@MANISH-K-07
Copy link
Contributor Author

MANISH-K-07 commented Mar 28, 2024

@rnveach , I am on par with what you said, I think @romani wants to close this issue in a subtle way unlike the other easy issues which are filtered by label. I have no problem sending multiple PRs at once like you said as long as the fix gets merged :)

Is this commit good for merge ?
I'm rebasing on master..

@MANISH-K-07
Copy link
Contributor Author

https://github.com/checkstyle/checkstyle/actions/runs/8467334447/job/23197981375#step:4:2400

[INFO] ------------------------------------------------------------------------
------------ grep of linkcheck.html--BEGIN
Checking internal (checkstyle website) and external links.
--- .ci-temp/linkcheck-suppressions-sorted.txt	2024-03-28 12:45:20.802042203 +0000
+++ .ci-temp/linkcheck-errors-sorted.txt	2024-03-28 12:45:20.802042203 +0000
@@ -1,3 +1,5 @@
+<a class="externalLink" href="https://www.baeldung.com/java-double-brace-initialization">https://www.baeldung.com/java-double-brace-initialization</a>: 403 Forbidden
+<a class="externalLink" href="https://www.baeldung.com/java-double-brace-initialization">https://www.baeldung.com/java-double-brace-initialization</a>: 403 Forbidden
 <a href="AbstractExpressionHandler.html#checkChildren(com.puppycrawl.tools.checkstyle.api.DetailAST,int%5B%5D,com.puppycrawl.tools.checkstyle.checks.indentation.IndentLevel,boolean,boolean)">AbstractExpressionHandler.html#checkChildren(com.puppycrawl.tools.checkstyle.api.DetailAST,int%5B%5D,com.puppycrawl.tools.checkstyle.checks.indentation.IndentLevel,boolean,boolean)</a>: doesn't exist.
 <a href="AbstractExpressionHandler.html#checkChildren(com.puppycrawl.tools.checkstyle.api.DetailAST,int%5B%5D,com.puppycrawl.tools.checkstyle.checks.indentation.IndentLevel,boolean,boolean)">AbstractExpressionHandler.html#checkChildren(com.puppycrawl.tools.checkstyle.api.DetailAST,int%5B%5D,com.puppycrawl.tools.checkstyle.checks.indentation.IndentLevel,boolean,boolean)</a>: doesn't exist.
 <a href="AbstractExpressionHandler.html#checkChildren(com.puppycrawl.tools.checkstyle.api.DetailAST,int%5B%5D,com.puppycrawl.tools.checkstyle.checks.indentation.IndentLevel,boolean,boolean)">AbstractExpressionHandler.html#checkChildren(com.puppycrawl.tools.checkstyle.api.DetailAST,int%5B%5D,com.puppycrawl.tools.checkstyle.checks.indentation.IndentLevel,boolean,boolean)</a>: doesn't exist.
------------ grep of linkcheck.html--END

@romani , should we be concerned about this ? I'm not sure where this got induced 😕

@romani
Copy link
Member

romani commented Mar 28, 2024

I am not sure, but if this not making CI red, we can ignore it for a bit, sometime web links are not stable.

@MANISH-K-07
Copy link
Contributor Author

MANISH-K-07 commented Mar 28, 2024

I am not sure, but if this not making CI red, we can ignore it for a bit, sometime web links are not stable.

@romani , unfortunately CI Check no broken links / check_issues (push) is red in master

image

I don't think any change of this commit introduced broken links or any links in general.
We need to find out source of these links and see if we can remove them from our code and until then suppress these, I believe

How do you want to proceed?

@romani
Copy link
Member

romani commented Mar 28, 2024

If this problem stays for longer, we can disable link-check-plugin to check such link, there is already bunch of such excludes in pom.xml

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.

None yet

3 participants