Updated docs: Feature branches, Recipes #624

langpavel · 2016-04-26T06:48:12Z

Updated docs, documented feature branches, added recipes

Closes #623, #612, #610, #595, #173 – all Redux issues.

tobiasmuehl · 2016-04-27T10:16:27Z

CONTRIBUTING.md

@@ -49,6 +49,7 @@ forked repo, check that it meets these guidelines:
 * [Squash](http://stackoverflow.com/questions/5189560/squash-my-last-x-commits-together-using-git)
  your commits into one for each PR.
 * Run `npm test` to make sure that your code style is OK and there are no any regression bugs. 
+* If you contributing to opt-in feature, prefix your PR title with `[feature/...]` tag.


When contributing to an opt-in feature, prefix your PR with the ...
When contributing to an opt-in feature, apply the .. tag as a prefix to your PR title
I'm not sure whether "if" or "when" is correct

I like When contributing to an opt-in feature, apply the .. tag as a prefix to your PR title

bravo-kernel · 2016-04-27T10:49:02Z

Please 📯 when suggestions by @tobiasmuehl have been processed so I can do a final check.

tomByrer · 2016-04-27T19:04:29Z

README.md

@@ -52,6 +52,25 @@ expenses via [OpenCollective](https://opencollective.com/react-starter-kit) or
  <img src="https://opencollective.com/static/images/become_backer.svg" width="64" height="64" alt="">
 </a>

+### Feature branches
+
+Some features aren't provided by default, but you can opt-in to them.


...but you can optionally add them.

@tomByrer Thanks, updated

langpavel · 2016-04-27T19:14:35Z

@bravo-kernel Now it should be ready

tomByrer · 2016-04-27T19:24:12Z

@bravo-kernel & @langpavel I believe that adding features via a git branch is a very bad idea, and is too unusual. Merging into existing code is hard to do. Plus, when people branch to add/PR their changes, they'll have to merge & unmerge 2+ other branches to test also?!?

If a repo wants to add additional features, they usually either include the code in part of the main branch, and use a flag/gulpscript/option to turn it on or off. Or they have a separate repo for the new feature, & npm install --save <package name>.

Plus what happens when someone forks your branch? feature/redux/newer-feature/fix-flux?

bravo-kernel · 2016-04-27T19:53:00Z

It seems I can only github-comment on lines that have been changed and would prefer to proof-read the full documentation. Anybody know a workaround?

langpavel · 2016-04-27T22:44:17Z

I believe that adding features via a git branch is a very bad idea, and is too unusual. Merging into existing code is hard to do.

When you checkout master and merge feature, why you cannot merge master again? It should work.
When you resolve merge conflict one time it is just resolved forever. Merging into existing code is always hard to do. There will be I believe feature/relay in the future

Plus, when people branch to add/PR their changes, they'll have to merge & unmerge 2+ other branches to test also?!?

Why? If your work is based on feature branch, you just want to merge it in feature. If it is useful for someone using different branch then he can git cherry-pick it as far as conflict happen.

If a repo wants to add additional features, they usually either include the code in part of the main branch, and use a flag/gulpscript/option to turn it on or off. Or they have a separate repo for the new feature, & npm install --save .

No one of your suggestions does solve the situation of competiting decisions. Separate multiple repos is even more fragile to maintain and it breaks community.

Plus what happens when someone forks your branch? feature/redux/newer-feature/fix-flux?

Actually as you know, feature/react-intl is based on feature/redux.

If request for master/react-intl will be there, It can be done! And it can be done pretty:

branch from master: git checkout master && git checkout -b master/react-intl
code it, if possible cherry-pick what possible, clean code, make the HOC if reasonable
create a PR against master, then can be decided if will be merged to master or as feature
checkout to feature/react-intl and merge feature/bare, resolve conflicts
Make new PR against feature/react-intl

Result of this is that all the branches here can be merged without conflicts!

If there will be feature/relay and request for feature/relay/react-intl it can be done too! Just follow idea.

I know this is an unproved theory but it may work. General idea is make react-starter-kit real kit with options and choices.

Please @koistya, comment on this idea.

langpavel · 2016-04-27T23:12:15Z

@tomByrer Described scenario can have interesting side effects:

You have three branches: one implementing Relay, other implementing Redux. Third implementing Intl. The third, bare Intl needs user provided code to works, pass messages into IntlProvider.

Assume two new branches - Redux+Intl and Relay+Intl. Each provide own mechanism to feed IntlProvider with data.

Assume that some good man improve Intl implementation and PR will be accepted. Now it can be merged by maintainers (or possibly by a bot when no conflict) to feature branches!

Users going with Relay and users going with Redux will be satisfied!

Even more: You can merge (and I wish try this) Redux and Relay. Now it will be interesting more:
Which implementation of ???-Intl will be used? :-) Definitely conflict here, but resolve it once and you can merge all updates from all branches after.

I'm wrong? Let me know!

tobiasmuehl · 2016-04-28T08:06:45Z

@bravo-kernel, Viewing the whole files is possible. Go to Files changed and click these:

bravo-kernel · 2016-04-28T12:20:55Z

README.md

+To do so, simply merge the corresponding feature branch.
+These branches should be in sync with master and all other features branches
+so there should not be any merging conflicts.
+If conflicts occure, please open an issue and report to us.


occur instead of occure

Fixed now, Thanks :-)

bravo-kernel · 2016-04-28T12:59:17Z

docs/recipes/feature-react-intl.md

+[`<FormattedNumber>`](https://github.com/yahoo/react-intl/wiki/Components#formattednumber)
+[`<FormattedPlural>`](https://github.com/yahoo/react-intl/wiki/Components#formattedplural)
+
+- Do not use `<FormattedHTMLMessage>` if possible, see how to use *Rich Text Formatting* with


If possible, do not use ..., see how to

bravo-kernel · 2016-04-28T13:12:23Z

@tobiasmuehl the problem is with not being able to add comments to the lines that show after expanding (the grey-colored lines). Would be nice but not being able to makes sense to me git-wise.

@langpavel my review is done, looks fine to me

bravo-kernel · 2016-04-28T14:40:58Z

CONTRIBUTING.md

@@ -49,6 +49,7 @@ forked repo, check that it meets these guidelines:
 * [Squash](http://stackoverflow.com/questions/5189560/squash-my-last-x-commits-together-using-git)


I can see it, in the left pane. Interesting

I don't understand you what do you mean?

tobiasmuehl · 2016-04-28T15:04:57Z

@bravo-kernel I tried if there's some way to make line comments using Chrome Dev Tools. The position on the server is handled by the data-position attribute on span.add-line-comment element which is relative to first commentable line (values 1..n). It's therefore impossible to comment on other lines. It would however be possible to develop a browser extension that fakes those comments by prefixing the line inside the comment and just putting the comment on a random line. That's not worth my effort only for this PR though.

bravo-kernel · 2016-04-28T15:08:04Z

@tobiasmuehl thanks for investigating, great find and no problem at.. you are absolutely right, not worth the effort. 👏

@tobiasmuehl

Closes kriasoft#623 Closes kriasoft#612 Closes kriasoft#610 Closes kriasoft#595 Closes kriasoft#173 Many thanks to @tobiasmuehl and @tomByrer and @bravo_kernel for reviewing.

langpavel · 2016-04-28T16:06:01Z

@koistya Reviewed by @tobiasmuehl, @tomByrer and @bravo-kernel.
Now it should be perfect 😉

langpavel · 2016-05-03T18:01:33Z

@koistya @frenzzy Can you please review and merge? There is arising interest into redux on gitter.

tobiasmuehl reviewed Apr 27, 2016
View reviewed changes

langpavel force-pushed the docs branch 2 times, most recently from 9a27a7f to a9daa30 Compare April 27, 2016 11:26

tomByrer reviewed Apr 27, 2016
View reviewed changes

langpavel force-pushed the docs branch from a9daa30 to 0eb6b28 Compare April 27, 2016 19:10

bravo-kernel reviewed Apr 28, 2016
View reviewed changes

langpavel force-pushed the docs branch from 0eb6b28 to 12d24dd Compare April 28, 2016 12:34

bravo-kernel reviewed Apr 28, 2016
View reviewed changes

Updated docs

ba597af

Closes kriasoft#623 Closes kriasoft#612 Closes kriasoft#610 Closes kriasoft#595 Closes kriasoft#173 Many thanks to @tobiasmuehl and @tomByrer and @bravo_kernel for reviewing.

langpavel force-pushed the docs branch from 12d24dd to ba597af Compare April 28, 2016 16:01

langpavel changed the title ~~Updated docs, documented feature branches, added recipes~~ Updated docs: Feature branches, Recipes Apr 28, 2016

koistya merged commit ba597af into kriasoft:master May 3, 2016

langpavel added the docs label Aug 2, 2016

This was referenced Jan 20, 2022

[Snyk] Fix for 3 vulnerabilities lamo2k123/react-starter-kit#30

Open

[Snyk] Fix for 3 vulnerabilities SARAN-thala/react-starter-kit#23

Open

[Snyk] Fix for 3 vulnerabilities priyacake/react-starter-kit#36

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated docs: Feature branches, Recipes #624

Updated docs: Feature branches, Recipes #624

langpavel commented Apr 26, 2016 •

edited

Loading

tobiasmuehl Apr 27, 2016

langpavel Apr 27, 2016

bravo-kernel commented Apr 27, 2016

tomByrer Apr 27, 2016

langpavel Apr 27, 2016

langpavel commented Apr 27, 2016

tomByrer commented Apr 27, 2016

bravo-kernel commented Apr 27, 2016

langpavel commented Apr 27, 2016

langpavel commented Apr 27, 2016

tobiasmuehl commented Apr 28, 2016

bravo-kernel Apr 28, 2016

langpavel Apr 28, 2016

bravo-kernel Apr 28, 2016

bravo-kernel commented Apr 28, 2016 •

edited

Loading

bravo-kernel Apr 28, 2016

langpavel Apr 28, 2016

tobiasmuehl commented Apr 28, 2016 •

edited

Loading

bravo-kernel commented Apr 28, 2016

langpavel commented Apr 28, 2016 •

edited

Loading

langpavel commented May 3, 2016

		@@ -49,6 +49,7 @@ forked repo, check that it meets these guidelines:
		* [Squash](http://stackoverflow.com/questions/5189560/squash-my-last-x-commits-together-using-git)

Updated docs: Feature branches, Recipes #624

Updated docs: Feature branches, Recipes #624

Conversation

langpavel commented Apr 26, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bravo-kernel commented Apr 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

langpavel commented Apr 27, 2016

tomByrer commented Apr 27, 2016

bravo-kernel commented Apr 27, 2016

langpavel commented Apr 27, 2016

langpavel commented Apr 27, 2016

tobiasmuehl commented Apr 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bravo-kernel commented Apr 28, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasmuehl commented Apr 28, 2016 • edited Loading

bravo-kernel commented Apr 28, 2016

langpavel commented Apr 28, 2016 • edited Loading

langpavel commented May 3, 2016

langpavel commented Apr 26, 2016 •

edited

Loading

bravo-kernel commented Apr 28, 2016 •

edited

Loading

tobiasmuehl commented Apr 28, 2016 •

edited

Loading

langpavel commented Apr 28, 2016 •

edited

Loading