Package refactoring #616

GiBg1aN · 2017-07-30T18:05:55Z

package ru.mystamps.web.model renamed to ru.mystamps.web.controller.dto
package ru.mystamps.web.validation.jsr303 renamed to ru.mystamps.web.support.beanvalidation

Addressed to #591

php-coder · 2017-07-30T18:51:07Z

@GiBg1aN Hi! Thank you for your contribution!

Could you split this into 2 commits (to have each renaming will be in the separate commit)? And also update the commits description to not mention the original issue (and we won't have spam comments in the issue from the temporary commits).

php-coder · 2017-07-30T18:54:44Z

Also don't forget to update here:

src/main/config/checkstyle-suppressions.xml:    <suppress checks="HideUtilityClassConstructor" files="ru.mystamps.web.model" />

php-coder · 2017-07-30T18:56:55Z

There are also translations for the validators that should be updated as well: see src/main/resources/ru/mystamps/i18n/ValidationMessages.properties and src/main/resources/ru/mystamps/i18n/ValidationMessages_ru.properties

GiBg1aN · 2017-07-30T20:38:57Z

@php-coder thank you for answering! I just splitted it in two commits and properly distributed the 3 missing files to them.

mystamps-bot · 2017-07-30T20:43:12Z

	2 Warnings
⚠️	danger check: pull request contains 2 commits while most of the cases it should have only one. If it’s not a special case you should squash the commits into single one. You can read how to do it here: https://davidwalsh.name/squash-commits-git But be careful because it can destroy all your changes!
⚠️	danger check: branch `rename` does not comply with our best practices. Branch name should use the following scheme: `ghXXX_meaningful-name` where `XXX` is an issue number. Next time, please, use this scheme :)

Generated by 🚫 Danger

php-coder · 2017-07-30T20:48:48Z

src/main/resources/ru/mystamps/i18n/ValidationMessages_ru.properties

@@ -1,50 +1,50 @@
-javax.validation.constraints.NotNull.message = Поле обязательно для заполнения


Could you modify this file without fully decoding it?

Ok, I'm gonna try the replacement with another editor

If you have sed then you can do this with the command: sed -i 's|validation\.jsr303|support.beanvalidation|' src/main/resources/ru/mystamps/i18n/ValidationMessages.properties

php-coder · 2017-07-30T20:50:20Z

@GiBg1aN Please, look at the errors from our bot and try to fix them.

php-coder · 2017-07-30T20:59:43Z

Couldn’t find pom.log. sortpom-maven-plugin result is unknown

This and other similar warnings just have been fixed in the commit 1094952 They will gone if you rebase your branch against the latest master. But it's not required.

php-coder · 2017-07-30T21:04:27Z

@GiBg1aN I see that you use your nickname as an author for the commits. Generally I advise to use a real name (but it's not required of course). So, consider updating the author value in the commits (git commit user.name "John Dow" && git commit --am --reset-author).

coveralls · 2017-07-30T21:59:34Z

Coverage remained the same at 75.7% when pulling 4c29efa on GiBg1aN:rename into 1094952 on php-coder:master.

php-coder · 2017-07-30T22:03:38Z

src/main/java/ru/mystamps/web/support/beanvalidation/ReleaseDateIsNotInFuture.java

 public @interface ReleaseDateIsNotInFuture {
-	String message() default "{ru.mystamps.web.validation.jsr303.ReleaseDateIsNotInFuture.message}";
+	String message() default "{ru.mystamps.web.support.beanvalidation"
+		+ ".ReleaseDateIsNotInFuture.message}";


Why do we need this? If you were trying to fix CheckStyle complains about a long line, then in this particular case, it's better to ignore them with a special comment:

// CheckStyle: ignore LineLength for next 1 line

php-coder · 2017-07-30T22:08:54Z

src/main/java/ru/mystamps/web/controller/CategoryController.java

 import ru.mystamps.web.controller.editor.ReplaceRepeatingSpacesEditor;
 import ru.mystamps.web.dao.dto.LinkEntityDto;
 import ru.mystamps.web.dao.dto.SeriesInfoDto;
-import ru.mystamps.web.controller.dto.AddCategoryForm;


The changes like that is irrelevant to the validation package renaming and should be part of the previous commit instead. I'm trying to have the commits that are "atomic" and modifies only one particular thing at time.

Could you move such changes from this commit to the previous (it requires some diff/patch/git rebase magic)?

php-coder · 2017-07-30T22:10:13Z

src/main/java/ru/mystamps/web/support/beanvalidation/ExistingActivationKey.java

 public @interface ExistingActivationKey {
-	String message() default "{ru.mystamps.web.validation.jsr303.ExistingActivationKey.message}";
+	String message() default "{ru.mystamps.web.support.beanvalidation"
+		+ ".ExistingActivationKey.message}";


This should be a single line.

php-coder · 2017-07-30T22:11:57Z

Thank you again!
Overall it looks good but some changes are required to make it mergeable.

php-coder · 2017-07-30T22:19:26Z

Could you also remove these lines as they not needed anymore:

mystamps/src/main/config/findbugs-filter.xml

Lines 11 to 19 in 1094952

    
           	<Match> 
        
           		<!-- 
        
           			Our DTO classes expose it's internal representation. 
        
           			Most of the time because they're using Date class and 
        
           			storing data in byte[]. 
        
           		--> 
        
           		<Class name="~.*\.model\..*" /> 
        
           		<Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2" /> 
        
           	</Match>

(they should go to the appropriate commit).

package ru.mystamps.web.model renamed to ru.mystamps.web.controller.dto

package ru.mystamps.web.validation.jsr303 renamed to ru.mystamps.web.support.beanvalidation

coveralls · 2017-07-31T07:42:31Z

Coverage remained the same at 75.7% when pulling bba4aef on GiBg1aN:rename into 62e540d on php-coder:master.

GiBg1aN · 2017-07-31T07:46:53Z

merged master commits
file editing grouped in the proper commit
removed unused code from findbugs-filter.xml
longline warnings suppressed

I hope that now everything is ok

php-coder · 2017-07-31T09:05:26Z

src/main/java/ru/mystamps/web/controller/dto/ActivateAccountForm.java

-import ru.mystamps.web.validation.jsr303.FieldsMatch;
-import ru.mystamps.web.validation.jsr303.FieldsMismatch;
-import ru.mystamps.web.validation.jsr303.UniqueLogin;
+


I removed this extra blank line in eede8a2 commit.

php-coder · 2017-07-31T09:06:02Z

src/main/java/ru/mystamps/web/support/beanvalidation/ExistingActivationKey.java

 @Documented
 public @interface ExistingActivationKey {
-	String message() default "{ru.mystamps.web.validation.jsr303.ExistingActivationKey.message}";
+        // CheckStyle: ignore LineLength for next 1 line


I fixed indentation in 348d637 commit.

php-coder · 2017-07-31T09:08:48Z

Merged in e19367e and ba82718 commits.

GiBg1aN · 2017-07-31T09:11:45Z

Thank you for your time and for your tips, this was my first PR

php-coder · 2017-07-31T09:14:20Z

@GiBg1aN I've just added you to our contributor list: https://github.com/php-coder/mystamps/wiki/team-members

Do you want to do more fixes/improvements and join our team?

GiBg1aN · 2017-07-31T09:47:33Z

Thank you for your proposal, maybe in the future ;)

Can I delete this branch? I don't know why but it says that there are unmerged commits

php-coder · 2017-07-31T10:02:08Z

Thank you for your proposal, maybe in the future ;)

Ok

Can I delete this branch?

Yes, sure.

I don't know why but it says that there are unmerged commits

Yes, that's because I didn't merge the commits but cherry-picked them (and also modified their descriptions).

GiBg1aN · 2017-07-31T10:04:23Z

Thanky you another time, maybe in the next days I'll take a look at your other issues and try to fix them.

php-coder · 2017-08-05T13:07:35Z

@GiBg1aN BTW how did you find this project?

GiBg1aN · 2017-08-05T19:25:40Z

I was looking for Java repos that had an issue labeled as "trivial"

php-coder reviewed Jul 30, 2017

View reviewed changes

php-coder changed the title ~~Package refactoring, issue #591~~ Package refactoring Jul 30, 2017

php-coder reviewed Jul 30, 2017

View reviewed changes

GiBg1aN added 2 commits July 31, 2017 09:18

Rename web.model package

f908b14

package ru.mystamps.web.model renamed to ru.mystamps.web.controller.dto

Rename web.validation.jsr303 package

bba4aef

package ru.mystamps.web.validation.jsr303 renamed to ru.mystamps.web.support.beanvalidation

php-coder reviewed Jul 31, 2017

View reviewed changes

php-coder mentioned this pull request Jul 31, 2017

Rename packages #591

Closed

php-coder closed this Jul 31, 2017

php-coder added the resolution/merged label Jul 31, 2017

GiBg1aN deleted the rename branch July 31, 2017 10:04

		@@ -1,50 +1,50 @@
		javax.validation.constraints.NotNull.message = Поле обязательно для заполнения

Package refactoring #616

Package refactoring #616

Uh oh!

Conversation

GiBg1aN commented Jul 30, 2017 • edited by php-coder Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

php-coder commented Jul 30, 2017

Uh oh!

php-coder commented Jul 30, 2017

Uh oh!

php-coder commented Jul 30, 2017

Uh oh!

GiBg1aN commented Jul 30, 2017

Uh oh!

mystamps-bot commented Jul 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

php-coder Jul 30, 2017

Choose a reason for hiding this comment

Uh oh!

GiBg1aN Jul 30, 2017

Choose a reason for hiding this comment

Uh oh!

php-coder Jul 30, 2017

Choose a reason for hiding this comment

Uh oh!

php-coder commented Jul 30, 2017

Uh oh!

php-coder commented Jul 30, 2017

Uh oh!

php-coder commented Jul 30, 2017

Uh oh!

coveralls commented Jul 30, 2017

Uh oh!

php-coder Jul 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

php-coder Jul 30, 2017

Choose a reason for hiding this comment

Uh oh!

php-coder Jul 30, 2017

Choose a reason for hiding this comment

Uh oh!

php-coder commented Jul 30, 2017

Uh oh!

php-coder commented Jul 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jul 31, 2017

Uh oh!

GiBg1aN commented Jul 31, 2017

Uh oh!

php-coder Jul 31, 2017

Choose a reason for hiding this comment

Uh oh!

php-coder Jul 31, 2017

Choose a reason for hiding this comment

Uh oh!

php-coder commented Jul 31, 2017

Uh oh!

GiBg1aN commented Jul 31, 2017

Uh oh!

php-coder commented Jul 31, 2017

Uh oh!

GiBg1aN commented Jul 31, 2017

Uh oh!

php-coder commented Jul 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GiBg1aN commented Jul 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

php-coder commented Aug 5, 2017

Uh oh!

GiBg1aN commented Aug 5, 2017

Uh oh!

Reviewers

Assignees

GiBg1aN commented Jul 30, 2017 •

edited by php-coder

Loading

mystamps-bot commented Jul 30, 2017 •

edited

Loading

php-coder Jul 30, 2017 •

edited

Loading

php-coder commented Jul 30, 2017 •

edited

Loading

php-coder commented Jul 31, 2017 •

edited

Loading

GiBg1aN commented Jul 31, 2017 •

edited

Loading