Skip to content

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

Merged
romani merged 1 commit into
checkstyle:masterfrom
shashwatj07:google-whitespaceafter
Apr 21, 2020
Merged

Issue #5624: Google Style Should Enforce Spaces after Commas#7988
romani merged 1 commit into
checkstyle:masterfrom
shashwatj07:google-whitespaceafter

Conversation

@shashwatj07

@shashwatj07 shashwatj07 commented Mar 29, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 suggested changes Apr 4, 2020
View reviewed changes
Comment thread ...ogle/checkstyle/test/chapter4formatting/rule462horizontalwhitespace/WhitespaceAfterTest.java Outdated
Comment thread src/main/resources/google_checks.xml Outdated
Comment thread src/main/resources/google_checks.xml Outdated
@shashwatj07 shashwatj07 force-pushed the google-whitespaceafter branch from 1312a60 to 084b6a3 Compare April 4, 2020 13:07
@pbludov pbludov assigned rnveach and unassigned pbludov Apr 4, 2020
@pbludov pbludov requested a review from rnveach April 4, 2020 13:34
@rnveach

rnveach commented Apr 4, 2020

Copy link
Copy Markdown
Contributor

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

rnveach
rnveach reviewed Apr 4, 2020
View reviewed changes
Comment thread src/main/resources/google_checks.xml Outdated
@shashwatj07

shashwatj07 commented Apr 5, 2020

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

rnveach commented Apr 6, 2020

Copy link
Copy Markdown
Contributor

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

shashwatj07 commented Apr 7, 2020

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

shashwatj07 commented Apr 8, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

@rnveach

rnveach commented Apr 8, 2020

Copy link
Copy Markdown
Contributor

@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

shashwatj07 commented Apr 8, 2020

Copy link
Copy Markdown
Contributor Author

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

rnveach
rnveach suggested changes Apr 13, 2020
View reviewed changes

@rnveach rnveach left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Final items.

Comment thread src/main/resources/google_checks.xml Outdated
Comment thread src/main/resources/google_checks.xml Outdated
Comment thread src/xdocs/google_style.xml Outdated
@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
@shashwatj07 shashwatj07 requested a review from rnveach April 14, 2020 07:20
@rnveach rnveach assigned romani and unassigned rnveach Apr 15, 2020
romani
romani requested changes Apr 18, 2020
View reviewed changes

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

items to improve:

Comment thread ...gle/checkstyle/test/chapter4formatting/rule462horizontalwhitespace/InputWhitespaceAfter.java
Comment thread src/main/resources/google_checks.xml Outdated
@shashwatj07 shashwatj07 mentioned this pull request Apr 18, 2020
Closed
@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
@shashwatj07 shashwatj07 force-pushed the google-whitespaceafter branch from c584625 to 53b5084 Compare April 20, 2020 14:08
@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

+2 more reviewers

@pbludov pbludov pbludov approved these changes

@rnveach rnveach rnveach approved these changes

Reviewers whose approvals may not affect merge requirements

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