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.

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

#2657 Feature/ alphabetic ordered i18n #3492

Merged

Conversation

defudef
Copy link
Contributor

@defudef defudef commented Sep 5, 2019

Related issues

closes #2657

Short description and why it's useful

When developer wants to commit changes including csv translations, lint-staged will automatically sort staged i18n files alphabetically.

Which environment this relates to

Check your case. In case of any doubts please read about Release Cycle
Feature

  • Test version (https://test.storefrontcloud.io) - this is a new feature or improvement for Vue Storefront. I've created branch from develop branch and want to merge it back to develop
  • RC version (https://next.storefrontcloud.io) - this is a stabilisation fix for Release Candidate of Vue Storefront. I've created branch from release branch and want to merge it back to release
  • Stable version (https://demo.storefrontcloud.io) - this is an important fix for current stable version. I've created branch from hotfix or master branch and want to merge it back to hotfix

Upgrade Notes and Changelog

  • No upgrade steps required (100% backward compatibility and no breaking changes)
  • I've updated the Upgrade notes and Changelog on how to port existing VS sites with this new feature

@pkarw
Copy link
Collaborator

pkarw commented Sep 5, 2019

@defudef please update the changelog

@pkarw pkarw requested a review from filrak September 5, 2019 14:18
@krecik
Copy link

krecik commented Sep 5, 2019

It would be nice, if all commits will be squashed. It's better to read history with more descriptive commit message.

@pkarw
Copy link
Collaborator

pkarw commented Sep 5, 2019

@krecik thats good point, we should consider it @filrak @lukeromanowicz @patzick we we’re not squashing commits since the very beginning of the project therefore I’m not sure about starting it right now as it will be not fair regarding contribution statistics for newcomers however not sure if anybody takes care

@krecik
Copy link

krecik commented Sep 5, 2019

Squashing should be done also properly - not only take all commits and make one.

But this one is small, so there should be one commit, and also all should be rebased on top of develop - cause in PR there are 2 merge commits (just to update codebase - it can be easily removed). In large projects, where code is changing frequently, this only pollute history, and making it harder to read.

@pkarw contribution statistics can be a problem, but i think better is to keep good quality, rather than large volume of commits :) Volume of contribution can be checked by changed lines of code, rather than number of commits.

My proposition for commit message for this change is:

Feature: Sorting alphabetically I18N tranlations

Configure lint to sort translations files before commit.
Initial sort of translations files.

A a rule of thumb always should be squashed commits like: "improvments after CR", "fix", "fix issue", "updated XXX.js" (what file is updated we see in commit, better is to add info what was changed - ex. i'm updating readme - better is to add: "Change info about number of intergrations", etc.) - there are not descriptive in any way.

@defudef
Copy link
Contributor Author

defudef commented Sep 6, 2019

I love the idea too!
IMO It could perfect fit with commit conventions as I mentioned in other issue.

I'm not a fan of long commit messages (event if they're squashed). Better short description and linked issue.

@krecik
Copy link

krecik commented Sep 6, 2019

I love the idea too!

Nice :D So just do it now :) Be the first one to implement the change and show others how this should look like.

@patzick
Copy link
Collaborator

patzick commented Sep 10, 2019

@krecik please create an RFC with this proposition. It needs to be discussed within community. With this occasion would be great to add a generator for changelog from commits, like conventional changelog or something like this :)

@patzick patzick merged commit 35631c7 into vuestorefront:develop Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants