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 #7641: Adding code examples for HiddenField check #9234

Merged
merged 1 commit into from Feb 13, 2021
Merged

Issue #7641: Adding code examples for HiddenField check #9234

merged 1 commit into from Feb 13, 2021

Conversation

aryaniiit002
Copy link
Contributor

Issue #7641: Adding code examples for HiddenField check

@aryaniiit002
Copy link
Contributor Author

aryaniiit002 commented Feb 3, 2021

Please review!
can i add Somefield in here and other words if come later?

@strkkk
Copy link
Member

strkkk commented Feb 5, 2021

@aryaniiit002

can i add Somefield in here and other words if come later?

It is better to avoid words in whitelist, there should be a good reason for adding them to whitelist. Here reason is not good - please change name to something else.
Also, please correct code:

  1. No need for main methods
  2. Make examples same for all configs to show user difference
  3. Format it properly (somewhere spaces are missing and left curly on next line)
  4. Do not use var as variable name, it is confusing for users since it is reserved type name in java10+

@strkkk
Copy link
Member

strkkk commented Feb 5, 2021

@aryaniiit002
I see that almost all my items are corrected, this is good.

Make examples same for all configs to show user difference

Please do that. Also I noticed that some examples do not have violation comments, only OK

Also CI is failing, please fix it.

@romani
Copy link
Member

romani commented Feb 8, 2021

Github, generate website

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
Copy link
Member

romani commented Feb 8, 2021

Github, generate website

@aryaniiit002
Copy link
Contributor Author

this is an experiment.
please ignore

@aryaniiit002
Copy link
Contributor Author

GitHub, generate report

@strkkk
Copy link
Member

strkkk commented Feb 8, 2021

@aryaniiit002 to generate report, you need to place all links it first post.
See https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#executing-generation

If something is not clear in doc, fixes are welcome

@aryaniiit002
Copy link
Contributor Author

aryaniiit002 commented Feb 8, 2021

first post means 1st comment?
so can i send new PR and do it there?

@strkkk
Copy link
Member

strkkk commented Feb 8, 2021

first post means 1st comment?

it means first post in PR, it is #9234 (comment)

so can i send new PR and do it there?

Yes, but I do not understand why you need diff report. It is only for code changes, but you branch only has changes for documentation

@aryaniiit002
Copy link
Contributor Author

i was kind of experimenting for something, i am working #7878 on this so i think may be regression might help basically my situation is similar to this #7982 (comment)

@strkkk
Copy link
Member

strkkk commented Feb 8, 2021

@aryaniiit002 you need to do it either

  1. In separate PR
  2. on your local machine - https://github.com/checkstyle/contribution/blob/master/checkstyle-tester/README_MANUAL_EXECUTION.md#executing-diffgroovy

To compare diff it takes branch from PR, but current branch has no code changes, only docs

@romani
Copy link
Member

romani commented Feb 8, 2021

@aryaniiit002 , you do not need regression diff report in this PR, we need web site.

@romani
Copy link
Member

romani commented Feb 8, 2021

Github, generate web site

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
Copy link
Member

romani commented Feb 12, 2021

Github, generate web site

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
Copy link
Member

romani commented Feb 13, 2021

Github, generate web site

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