Split and Organize Checkstyle inputs by Test for LocalFinalVariableName #4144

Closed
romani opened this Issue Apr 2, 2017 · 8 comments

Comments

Projects
None yet
3 participants
@romani
Member

romani commented Apr 2, 2017

subtask of #3891 .

do a copy of all test input files to separate folder for LocalFinalVariableName .
Not any other Check should use new inputs.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Apr 2, 2017

Member

pre-assigned to @kuri-leo .
@kuri-leo, if you agree on assignment ,please make comment "I am on it"

Member

romani commented Apr 2, 2017

pre-assigned to @kuri-leo .
@kuri-leo, if you agree on assignment ,please make comment "I am on it"

@kuri-leo

This comment has been minimized.

Show comment
Hide comment
@kuri-leo

kuri-leo Apr 2, 2017

Contributor

I am on it

Contributor

kuri-leo commented Apr 2, 2017

I am on it

@kuri-leo

This comment has been minimized.

Show comment
Hide comment
@kuri-leo

kuri-leo Apr 2, 2017

Contributor

@romani I'm sorry to disturb you but I have a bug, I've change all the name, and when I did the unit test , found messeages.properties was only copied to com.puppycrawl.tools.checkstyle.checks.naming but it should be in com.puppycrawl.tools.checkstyle.checks.naming.LocalFinalVariableName. So I'm not sure where I can change the code to fix it, could you please give me a hint?

The exception output is java.util.MissingResourceException: Can't find bundle for base name com.puppycrawl.tools.checkstyle.checks.naming.LocalFinalVariableName.messages, locale zh_CN

I got it, I misunderstood the task, now it has been solved, thanks!

Contributor

kuri-leo commented Apr 2, 2017

@romani I'm sorry to disturb you but I have a bug, I've change all the name, and when I did the unit test , found messeages.properties was only copied to com.puppycrawl.tools.checkstyle.checks.naming but it should be in com.puppycrawl.tools.checkstyle.checks.naming.LocalFinalVariableName. So I'm not sure where I can change the code to fix it, could you please give me a hint?

The exception output is java.util.MissingResourceException: Can't find bundle for base name com.puppycrawl.tools.checkstyle.checks.naming.LocalFinalVariableName.messages, locale zh_CN

I got it, I misunderstood the task, now it has been solved, thanks!

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Apr 3, 2017

Member

@Luolc , please take a look at this zh_CN localization problem .

Member

romani commented Apr 3, 2017

@Luolc , please take a look at this zh_CN localization problem .

@Luolc

This comment has been minimized.

Show comment
Hide comment
@Luolc

Luolc Apr 3, 2017

Contributor

@romani Sure. :)

@kuri-leo
You moved the UT class to a sub-folder in your first commit: kuri-leo@09a0495#diff-5032c49975b461d6d27d0ea29b31328d. You just need to split the inputs, instead of the test itself.

Your latest commit runs well now.

BTW, by my experience in the past few days, here are some suggestions before you create PR:

  1. Squash all your commits into one.
  2. IMO, the name of inputs would not be end with "Test". @romani and other maintainers will confirm that.
Contributor

Luolc commented Apr 3, 2017

@romani Sure. :)

@kuri-leo
You moved the UT class to a sub-folder in your first commit: kuri-leo@09a0495#diff-5032c49975b461d6d27d0ea29b31328d. You just need to split the inputs, instead of the test itself.

Your latest commit runs well now.

BTW, by my experience in the past few days, here are some suggestions before you create PR:

  1. Squash all your commits into one.
  2. IMO, the name of inputs would not be end with "Test". @romani and other maintainers will confirm that.
@kuri-leo

This comment has been minimized.

Show comment
Hide comment
@kuri-leo

kuri-leo Apr 4, 2017

Contributor

@Luolc
Thank for your suggestions. This is my first time contributed to a open source project, I will pay more attention to the PR rules next time!

Contributor

kuri-leo commented Apr 4, 2017

@Luolc
Thank for your suggestions. This is my first time contributed to a open source project, I will pay more attention to the PR rules next time!

kuri-leo added a commit to kuri-leo/checkstyle that referenced this issue Apr 5, 2017

kuri-leo added a commit to kuri-leo/checkstyle that referenced this issue Apr 5, 2017

kuri-leo added a commit to kuri-leo/checkstyle that referenced this issue Apr 5, 2017

@kuri-leo

This comment has been minimized.

Show comment
Hide comment
@kuri-leo

kuri-leo Apr 6, 2017

Contributor

Hi @romani and @Vladlis ,I also want to help izishared to rename all checks to practice myself and make up my incorrect closing of PR.
So, can I rename all checks in com.puppycrawl.tools.checkstyle.checks.naming as a begin? :)

Contributor

kuri-leo commented Apr 6, 2017

Hi @romani and @Vladlis ,I also want to help izishared to rename all checks to practice myself and make up my incorrect closing of PR.
So, can I rename all checks in com.puppycrawl.tools.checkstyle.checks.naming as a begin? :)

romani added a commit that referenced this issue Apr 6, 2017

@romani romani added the miscellaneous label Apr 6, 2017

@romani romani added this to the 7.7 milestone Apr 6, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Apr 6, 2017

Member

fix is merged

Member

romani commented Apr 6, 2017

fix is merged

@romani romani closed this Apr 6, 2017

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