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

Issue #5624: Google Style Should Enforce Spaces after Commas #7988

Merged
merged 1 commit into from Apr 21, 2020
Merged

Issue #5624: Google Style Should Enforce Spaces after Commas #7988

merged 1 commit into from Apr 21, 2020

Conversation

shashwatj07
Copy link
Contributor

@shashwatj07 shashwatj07 commented Mar 29, 2020 •
edited

Closes #5624: Google Style Should Enforce Spaces after Commas
Diff report: https://shashwat70.github.io/diff2/index.html

@strkkk strkkk requested a review from pbludov April 4, 2020 10:25
@pbludov pbludov assigned rnveach and unassigned pbludov Apr 4, 2020
@pbludov pbludov requested a review from rnveach April 4, 2020 13:34
@rnveach
Copy link
Member

rnveach commented Apr 4, 2020

@shashwat70 Regression must be provided for google_checks.xml and the changes being introduced.

@shashwatj07
Copy link
Contributor Author

@shashwat70 Regression must be provided for google_checks.xml and the changes being introduced.

What should be put in the baseConfig and patchConfig properties while generating diff report? @rnveach

@rnveach
Copy link
Member

rnveach commented Apr 6, 2020

What should be put in the baseConfig and patchConfig properties while generating diff report?

https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/google_checks.xml from master and from your PR. This is what you changed in your PR.

@shashwatj07
Copy link
Contributor Author

I'm getting the following error while generating diff report. Please help. @rnveach
https://gist.github.com/shashwat70/6a3157121a8b3ac5de88e869bdf65adb

@OmarShawky1 OmarShawky1 mentioned this pull request Apr 7, 2020
Merged
@shashwatj07
Copy link
Contributor Author

shashwatj07 commented Apr 8, 2020 •
edited

@rnveach It's giving java.lang.OutOfMemoryError: Java Heap space also.

@rnveach
Copy link
Member

rnveach commented Apr 8, 2020

@shashwat70 Yea, looks like the problem is related to that.

https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#command-line-arguments
Please use mode single and trim down the config to just your additions to google_checks.xml.

@shashwatj07
Copy link
Contributor Author

Done. The report's link is in the PR description. @rnveach

rnveach
rnveach requested changes Apr 13, 2020
View reviewed changes
Copy link
Member

@rnveach rnveach left a comment

Choose a reason for hiding this comment

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

Final items.

src/main/resources/google_checks.xml Outdated Show resolved Hide resolved
src/main/resources/google_checks.xml Outdated Show resolved Hide resolved
src/xdocs/google_style.xml Outdated Show resolved Hide resolved
@rnveach rnveach requested a review from romani April 13, 2020 15:55
@shashwatj07 shashwatj07 force-pushed the google-whitespaceafter branch 2 times, most recently from ec2e5c0 to 0acf12a Compare April 14, 2020 07:19
@rnveach rnveach assigned romani and unassigned rnveach Apr 15, 2020
romani
romani requested changes Apr 18, 2020
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 to improve:

...gle/checkstyle/test/chapter4formatting/rule462horizontalwhitespace/InputWhitespaceAfter.java Outdated Show resolved Hide resolved
src/main/resources/google_checks.xml Outdated Show resolved Hide resolved
@shashwatj07 shashwatj07 mentioned this pull request Apr 18, 2020
Open
@shashwatj07 shashwatj07 force-pushed the google-whitespaceafter branch 2 times, most recently from 1c3c539 to c584625 Compare April 20, 2020 13:50
@shashwatj07 shashwatj07 requested a review from romani April 20, 2020 13:52
@romani romani merged commit deea241 into checkstyle:master Apr 21, 2020
@shashwatj07 shashwatj07 mentioned this pull request Apr 22, 2020
Merged
@shashwatj07 shashwatj07 deleted the google-whitespaceafter branch April 29, 2020 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romani romani romani approved these changes

@pbludov pbludov pbludov approved these changes

@rnveach rnveach rnveach approved these changes

Assignees

@romani romani

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Google Style Should Enforce Spaces after Commas
4 participants
@shashwatj07 @rnveach @romani @pbludov