Skip to content

Issue #13155: updated the VariableDeclarationUsageDistance check documentation. #16877

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
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Issue #13155: updated the VariableDeclarationUsageDistance check documentation. #16877

wants to merge 1 commit into from

Conversation

Z4ID-Reaper
Copy link
Contributor

@Z4ID-Reaper Z4ID-Reaper commented Apr 16, 2025 •
edited
Loading

references #13155 , #16881

preview of changes made

  • updated the example for block initialization as shown in example 2
  • updated the violation message comments
  • updated the description of allowedDistance , validateBetweenScopes properties

@Z4ID-Reaper Z4ID-Reaper force-pushed the checkdocUpdate branch 5 times, most recently from 97e7fd6 to dee19ef Compare April 16, 2025 12:03
@Z4ID-Reaper Z4ID-Reaper marked this pull request as ready for review April 16, 2025 12:05
@Z4ID-Reaper Z4ID-Reaper force-pushed the checkdocUpdate branch 3 times, most recently from 8d55867 to 2a5a3f3 Compare April 16, 2025 15:11
@Z4ID-Reaper
Copy link
Contributor Author

@Zopsss @romani please review, apologies for the delay.

also please see #16798 , once it has the approve label i can start working on it.

romani
romani requested changes Apr 16, 2025
View reviewed changes
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.

Items

@romani romani mentioned this pull request Apr 16, 2025
Open
@Z4ID-Reaper Z4ID-Reaper force-pushed the checkdocUpdate branch 3 times, most recently from d1628a1 to 17096ac Compare April 17, 2025 08:12
@Z4ID-Reaper Z4ID-Reaper requested a review from romani April 17, 2025 09:31
Pankraz76
Pankraz76 suggested changes Apr 18, 2025
View reviewed changes
Copy link

@Pankraz76 Pankraz76 left a comment

Choose a reason for hiding this comment

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

spelling ,

@romani
Copy link
Member

romani commented Apr 19, 2025

@Z4ID-Reaper , please fix spellings above.

@Z4ID-Reaper
Copy link
Contributor Author

Ill fix this and and other issues too , once i get to my laptop tomorrow.
I am out of station at this moment :)

@Z4ID-Reaper Z4ID-Reaper requested a review from Pankraz76 April 21, 2025 04:33
@Z4ID-Reaper
Copy link
Contributor Author

@romani @Pankraz76 updated , please review for merge.

Pankraz76
Pankraz76 suggested changes Apr 21, 2025
View reviewed changes
Copy link

@Pankraz76 Pankraz76 left a comment

Choose a reason for hiding this comment

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

last bit, then merge. +1

@Z4ID-Reaper Z4ID-Reaper reopened this Apr 21, 2025
@Z4ID-Reaper Z4ID-Reaper force-pushed the checkdocUpdate branch 2 times, most recently from d8fc91c to c463191 Compare April 21, 2025 16:37
@Z4ID-Reaper Z4ID-Reaper requested a review from Zopsss April 24, 2025 17:29
@Z4ID-Reaper
Copy link
Contributor Author

Github, generate site

@github-actions GitHub Actions
Copy link
Contributor

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/0847853_20250424174306/index.html

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/0847853_20250424174306/checks/coding/variabledeclarationusagedistance.html#Examples

Zopsss
Zopsss suggested changes Apr 25, 2025
View reviewed changes
Copy link
Member

@Zopsss Zopsss left a comment

Choose a reason for hiding this comment

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

Last:

@Z4ID-Reaper Z4ID-Reaper requested a review from Zopsss April 25, 2025 03:33
@Z4ID-Reaper
Copy link
Contributor Author

Z4ID-Reaper commented Apr 25, 2025 •
edited
Loading

@Zopsss
Screenshot 2025-04-25 090428

checkstyle/src/site/xdoc/checks/coding/variabledeclarationusagedistance.xml

Lines 30 to 31 in bb12f5f

<td>allowedDistance</td>
<td>Specify distance between declaration of variable and its first usage. Values should be greater than 0.</td>

should i change this to

Specify maximum distance between declaration of variable and its first usage. Values should be greater than 0.

@Z4ID-Reaper
Copy link
Contributor Author

Github, generate site

@github-actions GitHub Actions
Copy link
Contributor

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/5e76913_20250425041155/index.html

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/5e76913_20250425041155/checks/coding/variabledeclarationusagedistance.html#Examples

@Pankraz76
Copy link

Pankraz76 commented Apr 25, 2025 •
edited
Loading

Yes please, nice idea.

plural:
Specify maximum distance between declaration of variable and its first usage. Values should be greater than 0.
singular:
Specify maximum distance between declaration of variable and its first usage. Value should be greater than 0.
Specify maximum distance between declaration of variable and its first usage. Value should be positive.
Specify maximum distance between declaration of variable and its first usage. Value should be positive, greater than 0.
Specify maximum distance between declaration of variable and its first usage. Value should be positive (greater than 0).

there is only one param and not a collection, right? So we might want to apply singularity as well.

@Pankraz76
Copy link

Pankraz76 commented Apr 25, 2025 •
edited
Loading

May improve spelling by using some articles:
Specify the maximum distance between the declaration of a variable and its first usage. Value should be greater than 0.
And or make things more compact: declaration and its first usage
Specify the maximum distance between a variable´s declaration and its first usage. Value should be greater than 0.

@Z4ID-Reaper Z4ID-Reaper requested a review from Pankraz76 April 25, 2025 10:27
@github-actions GitHub Actions
Copy link
Contributor

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/5ebb88a_20250425103401/index.html

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/5ebb88a_20250425103401/checks/coding/variabledeclarationusagedistance.html#Properties

@Z4ID-Reaper
Copy link
Contributor Author

Github, generate site

Pankraz76
Pankraz76 approved these changes Apr 25, 2025
View reviewed changes
Copy link

@Pankraz76 Pankraz76 left a comment

Choose a reason for hiding this comment

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

awesome, thank you.

@github-actions GitHub Actions
Copy link
Contributor

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/9870ea5_20250425123020/index.html

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/9870ea5_20250425123020/checks/coding/variabledeclarationusagedistance.html#Properties

@Z4ID-Reaper Z4ID-Reaper mentioned this pull request Apr 25, 2025
Closed
@Z4ID-Reaper Z4ID-Reaper changed the title Issue #13155: update the docs example for block initialization methods Issue #13155: updated the VariableDeclarationUsageDistance check documentation. Apr 25, 2025
@Z4ID-Reaper
Copy link
Contributor Author

Github, generate site

@Z4ID-Reaper
Copy link
Contributor Author

Summary of changes made so far

  • updated the example for block initialization as shown in example 2
  • updated the violation message comments
  • updated the description of allowedDistance , validateBetweenScopes properties

this pr is in a position to close the following issues :- #13155 , #16881

from my side it's all good 👍.

@github-actions GitHub Actions
Copy link
Contributor

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/be7b628_20250425180855/index.html

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/be7b628_20250425180855/checks/coding/variabledeclarationusagedistance.html#Properties

@Zopsss
Copy link
Member

Zopsss commented Apr 26, 2025 •
edited
Loading

This PR was initially targeting only 1 issue right? @Z4ID-Reaper

Now it's targeting 2 issues.

@romani should we proceed with these changes? Or should we split the PR?

@Z4ID-Reaper
Copy link
Contributor Author

The second issue was very small work, and very related, to the point it arised from this PR only.
So i included both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pankraz76 Pankraz76 Pankraz76 approved these changes

@romani romani Awaiting requested review from romani

@Zopsss Zopsss Awaiting requested review from Zopsss

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Z4ID-Reaper @romani @Zopsss @Pankraz76 @smita1078