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

google_checks: update to most recent version of style guide (3 November 2016) #3755

Closed
romani opened this Issue Jan 24, 2017 · 9 comments

Comments

Projects
None yet
3 participants

@romani romani added the approved label Jan 24, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jan 28, 2017

Member

Our copy of the guide was 2016-07-12 which corresponds to commit google/styleguide@b075cb7 .

To see the differences between that version and current master, you can use the following URL:
google/styleguide@b075cb7...gh-pagesdiff-b6c9191ee5ccb7a5ef170e98e51d94d8

Member

rnveach commented Jan 28, 2017

Our copy of the guide was 2016-07-12 which corresponds to commit google/styleguide@b075cb7 .

To see the differences between that version and current master, you can use the following URL:
google/styleguide@b075cb7...gh-pagesdiff-b6c9191ee5ccb7a5ef170e98e51d94d8

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Mar 4, 2017

Contributor

I'm on it.

@romani
Is it the first step which should be addressed? Will #3888 be a separate update? What will we do if changes in #3888 will be completely controversial to changes in this issue?

Contributor

MEZk commented Mar 4, 2017

I'm on it.

@romani
Is it the first step which should be addressed? Will #3888 be a separate update? What will we do if changes in #3888 will be completely controversial to changes in this issue?

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Mar 4, 2017

Member

@MEZk Yes, both are separate. This issue should apply changes up to date specified in title.
#3888 will do from this date to the next one in that title.

What will we do if changes in #3888 will be completely controversial to changes in this issue?

IMO, let us know if there are any if you plan to work on the other. I haven't looked at both to see what is involved.

Member

rnveach commented Mar 4, 2017

@MEZk Yes, both are separate. This issue should apply changes up to date specified in title.
#3888 will do from this date to the next one in that title.

What will we do if changes in #3888 will be completely controversial to changes in this issue?

IMO, let us know if there are any if you plan to work on the other. I haven't looked at both to see what is involved.

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Mar 11, 2017

Contributor

@rnveach @romani
I reviewed all changes. Here is a quick summary:

Changes in Google Java Syle Guide which do not require additional changes in google_checks.xml or xdoc:

  1. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R40
  2. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R44
  3. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R225
  4. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R229
  5. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R240
  6. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R394
  7. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R460
  8. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R473
  9. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R671
  10. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R847
  11. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R1105
  12. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R671

Changes in Google Java Syle Guide which require changes in xdoc and/or the implementation of a new logic:

  1. Section 3.3.4 No static import for classes requires an implementation of a new option for AvoidStaticImportCheck. The main idea is to check whether there is an import of a class or there is an import of other static member. Now we check both
  2. Section 4.1.3 Empty blocks: may be concise requires new exmaples in IT area.
  3. This change will require the implementation of the logic which will process lambdas for NoLineWrapCheck or OperatorWrap. Now they do not treat lambdas.

The following change in Google Java Style Guide is not mandatory:

  1. This says that any line break may (but not must) be preceded by arbitrary whitespace followed by an implementation comment. So it is not mandatory.

Few more questions:

  1. How can we update the cached version of Google Java Style Guide stored at http://checkstyle.sourceforge.net/reports/google-java-style-20160712.html
  2. Should we create new issues for the rules which we cannot use now and link them as blockers at http://checkstyle.sourceforge.net/google_style.html till they are not resolved.
Contributor

MEZk commented Mar 11, 2017

@rnveach @romani
I reviewed all changes. Here is a quick summary:

Changes in Google Java Syle Guide which do not require additional changes in google_checks.xml or xdoc:

  1. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R40
  2. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R44
  3. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R225
  4. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R229
  5. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R240
  6. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R394
  7. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R460
  8. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R473
  9. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R671
  10. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R847
  11. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R1105
  12. google/styleguide@db0a263#diff-b6c9191ee5ccb7a5ef170e98e51d94d8R671

Changes in Google Java Syle Guide which require changes in xdoc and/or the implementation of a new logic:

  1. Section 3.3.4 No static import for classes requires an implementation of a new option for AvoidStaticImportCheck. The main idea is to check whether there is an import of a class or there is an import of other static member. Now we check both
  2. Section 4.1.3 Empty blocks: may be concise requires new exmaples in IT area.
  3. This change will require the implementation of the logic which will process lambdas for NoLineWrapCheck or OperatorWrap. Now they do not treat lambdas.

The following change in Google Java Style Guide is not mandatory:

  1. This says that any line break may (but not must) be preceded by arbitrary whitespace followed by an implementation comment. So it is not mandatory.

Few more questions:

  1. How can we update the cached version of Google Java Style Guide stored at http://checkstyle.sourceforge.net/reports/google-java-style-20160712.html
  2. Should we create new issues for the rules which we cannot use now and link them as blockers at http://checkstyle.sourceforge.net/google_style.html till they are not resolved.
@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Mar 11, 2017

Member

requires an implementation of a new option for AvoidStaticImportCheck. The main idea is to check whether there is an import of a class or there is an import of other static member.

I am ok with new option. You can split it out in separate issue.

This change will require the implementation of the logic which will process lambdas for NoLineWrapCheck or OperatorWrap. Now they do not treat lambdas.

separate to 2 issues.

says that any line break may (but not must) be preceded by arbitrary whitespace followed by an implementation comment. So it is not mandatory.

we need to ease our config.

How can we update the cached version of Google Java Style Guide stored at

I do this manually, as I receive PR that covers new version of document. As we can not cover all in one issue, it is ok to update coverage page with links to issue where it will be covered

Should we create new issues for the rules which we cannot use now and link them as blockers

not as blockers, but as future plans.

==========================

in this issue , I suggest to update coverage with links to issues when coverage become complete and I do cache of guide.

Member

romani commented Mar 11, 2017

requires an implementation of a new option for AvoidStaticImportCheck. The main idea is to check whether there is an import of a class or there is an import of other static member.

I am ok with new option. You can split it out in separate issue.

This change will require the implementation of the logic which will process lambdas for NoLineWrapCheck or OperatorWrap. Now they do not treat lambdas.

separate to 2 issues.

says that any line break may (but not must) be preceded by arbitrary whitespace followed by an implementation comment. So it is not mandatory.

we need to ease our config.

How can we update the cached version of Google Java Style Guide stored at

I do this manually, as I receive PR that covers new version of document. As we can not cover all in one issue, it is ok to update coverage page with links to issue where it will be covered

Should we create new issues for the rules which we cannot use now and link them as blockers

not as blockers, but as future plans.

==========================

in this issue , I suggest to update coverage with links to issues when coverage become complete and I do cache of guide.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Mar 11, 2017

Member

@romani

How can we update the cached version of Google Java Style Guide stored at

I do this manually

Is there any reason we can't put this as a pure html file in git repository, like under the site directory?
We wouldn't need a date linked file as we could see the changes directly in git.

Member

rnveach commented Mar 11, 2017

@romani

How can we update the cached version of Google Java Style Guide stored at

I do this manually

Is there any reason we can't put this as a pure html file in git repository, like under the site directory?
We wouldn't need a date linked file as we could see the changes directly in git.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Mar 12, 2017

Member

Is there any reason we can't put this as a pure html file in git repository, like under the site directory?

no good reason, it was required to be done ones manually (3 years ago), after that I updated it only ones. Now looks like Google team is restored interest to guide, so we could make it more automated. Please make it as separate issue.

Member

romani commented Mar 12, 2017

Is there any reason we can't put this as a pure html file in git repository, like under the site directory?

no good reason, it was required to be done ones manually (3 years ago), after that I updated it only ones. Now looks like Google team is restored interest to guide, so we could make it more automated. Please make it as separate issue.

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Mar 14, 2017

Contributor

@romani

I am ok with new option. You can split it out in separate issue.

Done. #4005

separate to 2 issues.

Created the new issue (#4006). I think that this should be checked by NoLineWrap only.

Contributor

MEZk commented Mar 14, 2017

@romani

I am ok with new option. You can split it out in separate issue.

Done. #4005

separate to 2 issues.

Created the new issue (#4006). I think that this should be checked by NoLineWrap only.

MEZk added a commit to MEZk/checkstyle that referenced this issue Mar 18, 2017

MEZk added a commit to MEZk/checkstyle that referenced this issue Mar 18, 2017

MEZk added a commit to MEZk/checkstyle that referenced this issue Mar 18, 2017

romani added a commit that referenced this issue Mar 19, 2017

@romani romani added this to the 7.7 milestone Mar 19, 2017

@romani romani closed this Mar 19, 2017

SergeyDzyuba added a commit to SergeyDzyuba/checkstyle that referenced this issue Mar 22, 2017

GitToasterhub added a commit to GitToasterhub/checkstyle that referenced this issue Mar 26, 2017

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