-
-
Notifications
You must be signed in to change notification settings - Fork 348
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
Community proposal: Git flow for Sming development #230
Comments
+1 |
1 similar comment
+1 |
agree |
looks to me that this will involve more work for admins(ones with write rights) to get our PRs merged into master. do we have enough resources now to handle the extra project management work? |
+1 something like this is the only way to have a stable Master or release branches while allowing work to be tested. At a minimum we should implement a development branch where any changes get merged into for testing...that way any new users that come along don't get unstable code. The usage of feature branches and pull requests allows an individual developer to continue to develop a feature self contained and then when its ready to be merged over to master, the whole branch can be rebased and merged in. |
Related to this. We now have three communication paths. Chat, Github and Forum.
|
@ADiea amount of administrative work is not more than it is right now. Feature pull requests will be merged same way, releases will be more clear. Which part do you think will have extra project-management? |
I agre about switching to gitflow. I'm afraid what it may require some additional resources. But I'm sure what Sming is really a large and serious project now, so it's requried and very important for all of us. |
super, then it's +1 from me also :) |
@kireevco : The new procedure that you are proposing has the following drawbacks :
and as such there is
And a remark
|
Who will test future brunches and when they will be merged to master? |
@flexiti it's your responsibility to ask people to test/code review features. So work with @alonewolfx2 @robotiko @hreintke @hrsavla and other contributors - I'm sure they would love to make a code review / test for your PR. The idea is to involve community much more, so changes will go out quicker. |
I think it will be in this way: changes now will be tested only by 1 or 2 person , and after ??? month ? 2? will be merged - we don't have testers, so the only efekts we will have , it is delayed comits in master branch :) but it is only my opinion |
Ok |
@hreintke welcome back, first of all,
I personally see that Richards, @alonewolfx2 and others who test don't have any issues with testing feature/rboot right now.
I've read all your (@flexiti) arguments, and I don't really see them valid so far. Also, I'm not sure why @flexiti is not taking a part in this discussion here. As an addition, other uses don't have issues submitting to develop (#276 #277 #272 #264 for instance) - it's just you who still remains grumpy :) PS. I'm not sure how this work, but you keep moving 'the date' |
LS, Only option to comment is open a new "generic issue" |
If you read the comments/questions during the process you notice the trouble.
Nice to hear that you think they are not valid. What is the opinion of others ?
I think they just are using a small part of the proposal and are not into the details complete procedure. (Yes, I know it is an assumption).
As you have seen, I had to send the message twice to get a response. I updated the date to use a date which is reasonable. Otherwise I would get the remark : "how can this be done in two days" |
LS, Branches/usage in "global sming repo"
In a real life example that would mean that that the following branches exist : Release_1.2.0 -> old release This would provide a procedure with :
In my opinion the above creates a process which has the benefits of stability and predictabilty, without being overly complex. Please comment ! |
I also feel this is a more clear way of doing things. It is true that I have not contributed lately due to focus on other areas of my hobby project but when I come back SMINGing I'd love to find things as straightforward as before :) So from my side +1 for @hreintke s proposal as it seems more clean and straightforward |
@kireevco write: I wrote my opinion and that's it. @kireevco write: yes it's true, but at the same time do not test other changes - the next month will deal with the next branch?. For my own purposes I made myself my own "develop" because I will not be test each branch separately - I do not have much time, have You?? In brief: If we take into account that, @kireevco metod can be adapted to this and @hreintke also. |
The only comment I would like to add is a decision needs to be made and move forward with more than one person being able to commit changes into develop and/or master. rboot has been in works for weeks now and is all disjointed because only one person can accept the PR. The feature branch is broken, but there is a PR for a fix specifically for that... because of the current process, there are multiple PRs and you can't really tell where things are. Its just frustrating when you update and then spend hours realizing that there are outstanding PRs that haven't been pulled into the feature branch in a timely manner. |
So, from what I see here, what is the main issue? Need to create a separate branch? As I mentioned before, let's skip this, unless we all feel like it is required for some feature. We will convert a PR to a feature branch, either by asking a committer to re-submit it or merging. @hreintke I don't see any essential difference, other than deemphasizing feature branches and keeping old releases. It looks simple, but in reality it is essentially the same. What are main differences you see that "change the game"?
@hreintke Can you please describe the whole flow you see it, starting from develop? I agree that we should minimize PRs to one, which is already happening when submitting to develop. |
@kireevco
(the name "future" will tempt occasional visitors to download and see how it work, because it does not ,they can discourage to the use sming) From time to time (of course, if the version is stable) you will be moved develop the master and gave another tag. This is important because a lot of people come here sometimes, rarely
Since then, All (contrary to appearances, we are not too many :)) we will test develop and assist in moving the test/rboot to develop. After this You tell us who will decide whether to reject the change, add to develop or create test branch (I hope this will be rare). This decision should be done a maximum of 1-2 days. |
Hi,
Right now after "having fun" and being involved with the /feature/rBoot integration/ testing I can tell that the biggest issue is the lack of real community and collaboration.
Result: Fast code iterations for stability with no repo control and the new flow ended in a total repo mess (I do support 200% @piperpilot comment) PR over PR .. not being merged, people reporting several times the same fixed issues because part of the BGFX PR where merged and some other were pending as PR, and not very clear vision in the branch way and how they relate with issues. I think all this is much worse and scarier for newcomers than previous method.
I'm with @hreintke and @flexiti in simplifying flows. In most projects head is not stable.. is development, if you want a stable one, get a release (as it was before). If want to add development one and tag and merge PRs for a release.. it is ok BUT we need more people with merge rights. Cannot have such delays. Keep is simple! 💃 |
@kireevco @flexity @robotiko @Others As promised a new description of my proposal for the workflow of sming. I will use a "real life example" to avoid generalized naming and terminology. In this situation there will be the following branches in Sming Main Repo :
Process for new functionality :
At this moment there will be the following branches in Sming main repo :
Of course as multiple PR's can be in progress/finished in parallel so Develop will hold all finished functionality/PR's When decision to go to a new release.
At this moment there will be the following in Sming Main Repo
If we additionally agree that :
There will be the an environment which has : Generic :
Users :
Developers :
Main Repo Committers :
Generic remarks :
And as always : Please comment, I will answer and explain any unclearity Happy reading |
Hi, Im' on a long vacation and I don't really have time and good internet. On Thu, Sep 17, 2015 at 1:52 PM, hreintke notifications@github.com wrote:
|
@hreintke Nice Ideas! Me personally like them. Main thing new to me and important base of whole your process is ability to fetch pull request (git fetch origin pull/111/head) so no extra branches on main repo, and consistent workflow of PR till it ready to be merged. +1 from me. |
@kireevco : @kireevco : @anakod :
|
"When websocket_client is tested, complete (ready), "Main_Repo_Admin/committer" merges PR-111 into develop. PR-111 is closed." it must be clarified, otherwise we will have again a lot of "branch" which now will be called the PR-111, etc. |
@hreintke - sounds very reasonable. I didn't realise it was possible to checkout a pull request, do many people know that? I like the idea of having a single PR for the feature (instead of a branch) which is accessible via the main repo yet updatable by the developer automatically from their fork. I was starting to think that the best way was just to go back to the old fashioned method of doing it all in your own fork and only PRing once complete, but the problem with that is when you are doing something non-trivial (rBoot integration has turned out to be a lot more complicated than expected - just the integration mind you, rBoot itself I'm pleased to say has been very well behaved) you need lots of testers and they aren't all that keen on switching to stuff running out of forks. Your way simply allows them to switch to the PR branch in the main repo, like the the current branch system, but giving the developer control over what's actually in there. |
Wow, I didn't know it was possible to check out a PR either. I believe if you update the branch that the PR is created against, it will all be included when it is merged in. I think this is a great solution. +1 for giving it a try! Soon! |
@raburton : @piperpilot
And on the github site, hidden in a help on working on stale pull request. If you, as experienced github users, can do a quick check on this option and feedback your results would be great. PS : Yes of course I did a first test myself 😄 |
@flexiti :
I started a (little) with that in my generic remarks.
But definitely needs to be extended. My thoughts on this however were : This is an open point in every Workfow we design. Let's finalize that discussion first before getting into the next discussion |
@hreintke https://github.com/hreintke Unfortunately I don't have any On Thu, Sep 17, 2015 at 4:37 PM, hreintke notifications@github.com wrote:
|
@kireevco @flexiti @robotiko @raburton @piperpilot @Others As promised a new description of my proposal for the workflow of sming. In "normal" situation there will be the following branches in Sming Main Repo :
And a Release in Github repository called "Sming_1.2.0" Process for new functionality to be added :
At that moment there will be the following branches in Sming main repo :
Of course as multiple PR's can be in progress/finished in parallel so Develop will hold all finished functionality/PR's When decision to go to a new release.
At that moment there will be the following in Sming Main Repo
Process for Bugfixes :
At that moment there will be the following in Sming Main Repo
Introduction of "Large Impact" functionality For these kind of PR's in general the following procedure is taken.
Additionally agree that :
Next steps
Please comment |
All of this sounds pretty sensible to me. I think we need to document 2 separate things...
If the process/documentation is too long and complex, we won't get people to contribute fixes. I don't think this is THAT complex...it just seems so due to the long explanation. |
@hreintke I fully trust you! |
In an ideal world, wouldn't releases happen every time a new feature/functionality is merged into develop, because at that point a feature is merged into develop, that code has already been vetted? I am just a bit unsure of the reasoning of having a separate develop and main branch, as the biggest workflow change is all PRs must start in a separate feature branch / PR. Simplified proposal
Upsides
Downside
|
@kireevco : Can we close this issue ? |
I don't know, feel free if you think so.
|
A process for contributions is agreed. |
I propose switching Sming framework to git flow. This will highly improve stability for the release, will allow us implement backwards-incompatible changes (like upgrading SDK) easily. Currently we can't do that at all without breaking Sming and causing issues for users.
This will also allow us to run automated tests.
master: Branch that contains latest production (stable) release
develop: Main development branch: contains all current new features
feature/websocket-client. Gets deleted when merged to develop: Branch for a new feature while in development process.
release/1.2.5: Special release branch that exists only when develop gets merged to master. This branch is used when a feature-set in develop is frozen, and the branch is used to send latest fixes, before it gets merged into master and published to sming/releases.
This will mean that all Contributors will have to submit a PR to a feature branch, it will be tested and then merged to develop for automated integration testing (via TravisCI, Jenkins or SemaphoreCI), as well as manual testing on a real device. All Commiters will need to submit a PR to develop branch, which then will be either merged by a Contributor or moved to a separate feature branch.
Git Flow
Committer
Contributor
Reviewing PRs & Manual Testing
Pull requests should be reviewed by peer Contributors, or @anakod. We should have a team of core-contributors that we trust. This should speed up development process and scale Sming Framework horizontally.
Real device testing preferred and encouraged, currently real device testing should be optional for features and mandatory for releases.
Continuous Integration & Continuous Testing
Every feature and every branch should have a continious build triggered on every commit. I will configure Travis similar to this demo project: https://travis-ci.org/sming-framework/Sming.
This will prevent broken builds before merging and allow us to do other automated things like style checks and doxygen comments validation as a part of a build, as well as unit tests eventually (TBD)
Releases
When develop branch is well tested by a majority of contributors, committers and users a stable release should be created. Release branch should be branched off a develop branch.
Using Continuous Integration that branch will be built into a distributable package, and a github release should be created. That release should contain descriptions of all features merged since the last release.
Testing on a real device is mandatory before making a release. Testing should be done by peer Contributors, and/or @anakod.
Repository Permissions
This is the most common approach for a git-flow:
http://nvie.com/posts/a-successful-git-branching-model/
Everyone who cares, let's chat :)
UPDATE:
Sep 6 Glad that we have people other than @anakod and myself who care. With some input I updated the proposal for both Contributors and Committers, fixed some mistakes, added CI clarification.
For now, until everyone is clear on what's happening, please submit PRs to develop branch same way you did to master. As soon as one's comfortable with creating a feature branch - feel free to do so.
The text was updated successfully, but these errors were encountered: