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

Community proposal: Git flow for Sming development #230

Closed
AutomationD opened this issue Aug 23, 2015 · 50 comments
Closed

Community proposal: Git flow for Sming development #230

AutomationD opened this issue Aug 23, 2015 · 50 comments

Comments

@AutomationD
Copy link
Contributor

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

  • Fork Sming repo
  • Create a local branch off the develop : feature/websocket-client
  • Build, test your code
  • Commit changes
  • Push your changes to your fork on github
  • Submit PR to the main Sming repo, to develop branch.

Contributor

  • Fork Sming repo
  • Create a local branch off the develop : feature/websocket-client
  • Build, test your code
  • Commit changes
  • Push your changes to anakod/sming repo and optionally your fork on github
  • Submit PR to the main Sming repo, to Sming/develop branch from Sming/feature/websocket-client.

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

  • Contributors should have write access to all branches
  • master, develop branches should be protected

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.

@robotiko
Copy link

+1

1 similar comment
@avr39-ripe
Copy link
Contributor

+1

@hreintke
Copy link
Contributor

agree

@ADiea
Copy link
Contributor

ADiea commented Aug 24, 2015

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?

@piperpilot
Copy link
Contributor

+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.

@hreintke
Copy link
Contributor

Related to this. We now have three communication paths. Chat, Github and Forum.
In all of those we have questions, issues, bugs,... And searching on earlier info is difficult.
Maybe we should get as much as possible to a differentiation of usage.

  • Chat : Used for quick information exchange, questions are expected to be answered/solved within a day. History deleted after one week. Mostly used by frequent users/developers.
  • Forum : Used for Information exchange on larger subjects, enhancement request and possible bugs.
    Also entry point for new users. History is kept. Well searchable.Maybe we need some subforums.
  • Github : Used for registration and follow up on Identified/Verified bugs and enhancement requests. Also for Push Requests.

@AutomationD
Copy link
Contributor Author

@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?

@anakod
Copy link
Member

anakod commented Aug 27, 2015

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.

@ADiea
Copy link
Contributor

ADiea commented Aug 28, 2015

super, then it's +1 from me also :)

@hreintke
Copy link
Contributor

hreintke commented Sep 6, 2015

@kireevco :
Also mentioned in gitter but here to keep visbility

The new procedure that you are proposing has the following drawbacks :

  • one cannot start a PR without having another (now you or anakod ?) creating a feature branch
  • after that another has to merge that into the feature branch
  • when merged the PR is closed
  • that means : It is not visible anymore as Pull request One has to look at which current feature branches are active

and as such there is

  • No "standard"way to comment on a PR
  • No-one (even the Original creator of the patch) can change or update the patch according to the comments
  • So if you want to update the PR you have to create a PR for the original PR and start the whole procedure again with the same drawbacks

And a remark

  • Should the feature branches be based on develop instead of master ?

@flexiti
Copy link
Contributor

flexiti commented Sep 6, 2015

Who will test future brunches and when they will be merged to master?

@AutomationD
Copy link
Contributor Author

@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.
(Before it was just @anakod who's time is very very limited).

The idea is to involve community much more, so changes will go out quicker.
Later we will add basic code syntax, climate and other testing automatically.

@AutomationD
Copy link
Contributor Author

@anakod please don't merge anything other than a release branch to master:
3be5c6e
28d6058

@flexiti
Copy link
Contributor

flexiti commented Sep 6, 2015

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

@AutomationD
Copy link
Contributor Author

@flexiti I don't think you're right.
If no one is available @anakod will review it same way as he did and approve a PR :)
Our goal is to improve code quality and overall feature set.
Let's chat tomorrow in gitter?

@flexiti
Copy link
Contributor

flexiti commented Sep 6, 2015

Ok

@AutomationD
Copy link
Contributor Author

@hreintke welcome back, first of all,

Need to create PR on PR because the original PR is closed and the committer cannot update/change
What is still missing here from @anakod is write permissions to the repo for Contributors. Either @flexiti should have them or submit a PR to develop

Probably because of the above, testers where unclear which version to use (or were available)
It is either feature/feature_name or develop if no feature branch is present. Disadvantage of having PR to develop is that you can't really test it until merging it

even that much that was suggested to used Richards github version, a version we don't know status and sming version of -> so what was really tested ?

I personally see that Richards, @alonewolfx2 and others who test don't have any issues with testing feature/rboot right now.

Unclearity on how to inplement, what and how should go where in sming (makefile, core,service,..)
Please elaborate how this is related to branches and git-flow?

Someone mentioning "too many branches to keep overview"
This is just personal opinion of someone. This is normal practice, and just get used to it. A lot of projects have much more branches, while we won't have much, maybe 5 at a time (the amount of features worked on.

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'

If you want to set such kinds of things, let's stick to one date.

@hreintke
Copy link
Contributor

LS,
Just noticed another drawback of the proposed way of working.
I wanted to submit a remark on the websocket client PR.
But.. the websocket client PR is closed because it is merged into the features/websocket-client branch.
But.. I cannot comment on a branch.

Only option to comment is open a new "generic issue"
This makes that there is no overall overview in current/past comments on a proposed PR

@hreintke
Copy link
Contributor

@kireevco :

I personally see that Richards, @alonewolfx2 and others who test don't have any issues with testing
feature/rboot right now.

If you read the comments/questions during the process you notice the trouble.
Why do they not comment on the procedure ? -> They are fully occupied by testing/updating and focusing on that

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.

Nice to hear that you think they are not valid. What is the opinion of others ?
That is exactly what I am trying to achieve.

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 :)

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).
And maybe... using "just a development branch"is a good idea.
We should get into this questioning instead of keeping fixed to one proposal.
BTW : I am not grumpy. I try to get a overall view of the proposals, send my remarks
and believe in a decision by consensus instead of a forced one man decision.

PS. I'm not sure how this work, but you keep moving 'the date'

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"

@hreintke
Copy link
Contributor

LS,
Seeing the discussion above, the way the rboot PR was handled and showing that I am prepared to think about a better way of getting Sming stable.
This is an alternate proposal for improved workflow.

Branches/usage in "global sming repo"

  • Develop
    All new features will PR to this branch
    PR's stay open until finished/ready for release
    Merge into develop means will be included in next release
  • For each release there will be a "Release branch"
    At regular intervals develop is merged into "Current release branch" creating "Next release branch"
    This finalizes the creation of release and "users" can use/pull this release as current.
  • Bugfixes are PR'ed against the "Current Release Branch"

In a real life example that would mean that that the following branches exist :

Release_1.2.0 -> old release
Release_1.3.0 -> old release
Release_1.4.0 -> current release + added bugfixes, PR's open for unfinished bugfixes
Develop -> Release_1.4.0 + added functionality for Release_1.5.0, PR's open for unfinished new features and bugfixes to merged functionality

This would provide a procedure with :

  • Known and agreed functionality for each release
  • Stable releases
  • Regular releases
  • Users know which functionality is available is stable in each release
  • Users are free to use current or old release -> upgrade in their projects timing
  • Users are free to use develop for reasonable stable new functionality
  • Developers without write access can initiate work/update/finish PR's without intervention from "main repo committers"
  • Developers have clear and complete overview of discussions/issues on PR's
  • Developers have clear overview of open "functionality in progress" and "Not yet complete bugfixes"

In my opinion the above creates a process which has the benefits of stability and predictabilty, without being overly complex.

Please comment !

@ADiea
Copy link
Contributor

ADiea commented Sep 15, 2015

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

@flexiti
Copy link
Contributor

flexiti commented Sep 15, 2015

@kireevco write:
"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."

I wrote my opinion and that's it.
Someone must decide, otherwise we will exchange views for years :)

@kireevco write:
"I personally see that Richards, @alonewolfx2 and others who test don't have any issues with testing feature/rboot right now"

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??
I am sure that today there is only rboot branch is tested by several people (3?), and the master branch by people who take sming. (btw. develop today is almost a master)
Other changes lie somewhere

In brief:
  - Too much barnch (acc. To me today ( according to what I wrote earlier) should be 'master' as this stable, develop as a development and only rboot as it essentially affects the entire project.
No matter if it will be called a tag or branch, it's important not to have this much.
Rapid tests additional branch (in this case rboot), and immediately transfer to develop.
All of which they use Sming take master, those who test Sming take the develop.

If we take into account that, @kireevco metod can be adapted to this and @hreintke also.

@piperpilot
Copy link
Contributor

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.

@AutomationD
Copy link
Contributor Author

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"?

For each release there will be a "Release branch"
At regular intervals develop is merged into "Current release branch" creating "Next release branch"
This finalizes the creation of release and "users" can use/pull this release as current.

@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.

@flexiti
Copy link
Contributor

flexiti commented Sep 16, 2015

@kireevco
if I suggest do it now:

  • Develop, is ok
  • The master can be,
  • Remove any branch except rboot and their contents move to develop
  • Rename future/rboot to test/rboot

(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
So soon they will be clear for them:

  • Develop, you know, the development version: there may be errors
  • Master (better Stable) version: this version people will download frequently
     (They can also select older versions by the tag's)
  • test/rboot, a test, only the persons concerned and informed will know what it is.
    (It's a temporary branch)

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.

@hreintke
Copy link
Contributor

@kireevco : @flexiti :
I do not have the possibility to reply in detail today but will do tomorrow (Thursday CET).

@robotiko
Copy link

Hi,
joining late to this party, I won't focus on the procedure but wanted to share some experience with the feature/rboot and an opinionated generalization from a high level perspective that might help and probably will get blamed for: :D

  1. Whatever method is chose, should be agreed and should be some transition time (as any change management project will tell you), and followed during adoption.
    The way the procedural change was managed was far from ideal.
    This issue got opened, some discussion, time goes by and suddenly there is no other way to work and everybody is requested to change. (Including some PR that were waiting for more than 2 month for PR).
    The effect was kind of chaotic: instead of a transition time and accept last PR as previous method, all contributors asked how to do it and had to learn the new procedure and reissue PR (extra work).
    A lot of noise and main problems started to pop up: no clear view of PR and issues for newcomers since most people is not used to browse branches to see status.
    As @hreintke pointed, not seeing contributions here doesn't mean that everyone agrees and everything is running smoothly. Sming is receiving most interaction from esp8266.com forum and Gitter in a different scale github issues.
    Since gitter is quite noisy, many contributors are no longer following it (this includes most of big wolves here :D) but is where most questions and issues pop up after the change and was not used to measure the impact of change.

  2. Should always be realistic about resources, efforts and goals:
    A procedure is just that. If resources are low, it should not be an overload.
    Community is growing with current lack of documentation and step learning curve, the number of contributors is quite reduced (normal) but they highly compromised. Should take advantage of it.

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.
There is no roadmap, and clear goals with polls, etc but the most relevant thing is the merge BOTTLENECK. This is really dangerous.

  1. Goals: Resource optimization and stability with agile approach.
    If the git workflows was introduced with this goal, the result was totally the opposite.
    Being so agile one side (contribution) and nothing at the other (merging), it became overkilling task and totally useless effort.
    Example: rBoot: quite relevant development, with many people willing to give it a try and we were lucky enough to have an heroic @raburton in fast mode deeply involved in getting rBoot into Sming.

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 had a "treasure map" with branch, issues and PR and merges.. but methodology/ tool should prevent from this flood. As proof, today after main rboot merge, many issues were starting to close reflecting that were due to using the wrong branch, not all the PR, etc
Have fun recovering gitter conversations from pasts days.. people checking out @raburton repo, some giving up.. some sharing how to cherry pick the different PR to make something stable, and even some people sharing to integrate different branches on top of the PR merges.. to be able to get a couple functionalities.
Stability and Agile.. maybe :D we ended testing part of changes with patches to not get crazier.
Next step in this procedure : send code zipped by email. (sarcastic).

I think all this is much worse and scarier for newcomers than previous method.
Not blaming anyone, just exposing.

  1. Empiric conclusion:
    -Sming community should be empowered. A week gap to merge BGFX PR is way too much and generated too much trouble. We are all too busy so there should be more people with merge rights.
    Contributors usually send fixes very fast once detected, but if times passes by this gets harder so should have fast reaction there.
    At least in the last couple of months, most Sming improvements (if not all), came from community contributors so don't see why they should not have more rights here (IMHO).
    -Not having a clear global picture of development status (all PR waiting to be merged, all issues and where they belong to, in the same place) is bad to control the project but also to show how active it is and potentially increase the community.
    -The past experience showed that most people is not used to the git flow adopted, and this change was justified to improve their experience.

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.
This is not a big project with tens of developers full time in different functionalities and advanced version planing deciding what to integrate in next version and what to keep for later.

Keep is simple! 💃

@hreintke
Copy link
Contributor

@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.
Current version of Sming is 1.2, example PR is websocket_client
No "history branches" present.


In this situation there will be the following branches in Sming Main Repo :

  • Release_1.2 -> Stable release
  • Develop -> After release identical to release, will be used for development of next release.

Process for new functionality :

  • @hrsavla writes a websocket_client in private environment
  • When appropriate (he is finished/want to discuss further progress with others) he initiates a PR against Develop Branch (PR numbered 111)
  • This PR is not merged into develop but stays open for further progress.
  • @Others can test/use this by : clone main_repo_branch develop, fetch PR-111 (git fetch origin pull/111/head), compile and test.
  • @Others will comment, find issues, propose updates to the PR, all will be communicated within the open PR-111 at Github
  • @hrsavla adapts the websocket_client code according to the remarks
  • @hrsavla pushes the additional commits to the outstanding (still open) PR-111
  • As soon as these commits are pushed, they are visible and usable by all (no action required by "main_repo_Admins/committers)
  • @Others can access/test/comment on the updated PR as mentioned above.
  • When websocket_client is tested, complete (ready), "Main_Repo_Admin/committer" merges PR-111 into develop. PR-111 is closed.

At this moment there will be the following branches in Sming main repo :

  • Release_1.2 -> Stable release
  • Develop -> Release_1.2 + added websocket_client (which is (close to) production ready)

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.

  • Add branch Release_1.3, based on Release_1.2
  • Merge Develop into Release_1.3
  • Developers rebase PR's on Release_1.3

At this moment there will be the following in Sming Main Repo

  • Branch Release_1.2 -> old stable release
  • Branch Release_1.3 -> new (latest) stable release
  • Branch Develop -> Bases on Release_1.3 .) ready for new functionality as above
  • Open PR's for unfinished new functionality based on Release_1.3

If we additionally agree that :

  • When development on a PR is finished (ready to merge) that will be done within a week.
  • There will be regular Sming releases (once a month ?)

There will be the an environment which has :

Generic :

  • Known and agreed functionality for each release
  • Stable releases
  • Regular releases
  • A Develop release which is close to stable and can be used/tested by users with less detailed (internal) knowledge of included changes.
  • The open PR's provide a complete overview of new functionality in progress

Users :

  • Know which functionality is available is stable in each release
  • Are free to use current or old release -> upgrade in their projects timing
  • Can use develop for reasonable stable new functionality

Developers :

  • Can initiate work/update/finish PR's without intervention from "main_repo_admin/committers"
  • Have clear and complete overview of discussions/issues on their PR's
  • Have clear overview of open "functionality in progress" and "Not yet complete bugfixes"
  • Have full access to complete and latest source code of new functionality without combining several PR's

Main Repo Committers :

  • Will do a merge to develop only when PR is finished
  • Will merge finished PR's to develop within agreed time interval
  • Have no time pressure to merge all updates on open PR's
  • Will do a merge to new release on regular intervals

Generic remarks :

  • The above is a procedure for most of the new functionality.
  • For extra large/complex new functionality a "one time" specific process can be created, probably with use of special additional branches.
  • Although less "time pressure" on PR updates, there is still the need for more "Main_Repo_Admin/Committers"
  • The steps above are compatible with Travis-CI which, when setup properly, will test open PR after each update.
  • The above is close to the process @flexity proposed (use of Branch instead of Tag). I stayed at branches as (I think) for the less experienced (github) user it is more common to download branches.
  • We should make sure that a "Release Branch" has the correct libsming library included. It should be usable without users compiling sming core.
  • I am thinking on an extension to the above by asking "Main_Repo_Admins/Committers" to provide a quick initial thought on the PR. That would prevent activity/work on PR's which concern/are against Sming philosophy.

And as always : Please comment, I will answer and explain any unclearity

Happy reading

@AutomationD
Copy link
Contributor Author

Hi,

Im' on a long vacation and I don't really have time and good internet.
@hreintke, everything looks good to me. Feel free to take a lead and
implement it!

On Thu, Sep 17, 2015 at 1:52 PM, hreintke notifications@github.com wrote:

@kireevco https://github.com/kireevco @flexity @robotiko
https://github.com/robotiko @Others https://github.com/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.
Current version of Sming is 1.2, example PR is websocket_client

No "history branches" present.

In this situation there will be the following branches in Sming Main Repo :

  • Release_1.2 -> Stable release
  • Develop -> After release identical to release, will be used for
    development of next release.

Process for new functionality :

  • @hrsavla https://github.com/hrsavla writes a websocket_client in
    private environment
  • When appropriate (he is finished/want to discuss further progress
    with others) he initiates a PR against Develop Branch (PR numbered 111)
  • This PR is not merged into develop but stays open for further
    progress.
  • @Others https://github.com/others can test/use this by : clone
    main_repo_branch develop, fetch PR-111 (git fetch origin pull/111/head),
    compile and test.
  • @Others https://github.com/others will comment, find issues,
    propose updates to the PR, all will be communicated within the open PR-111
    at Github
  • @hrsavla https://github.com/hrsavla adapts the websocket_client
    code according to the remarks
  • @hrsavla https://github.com/hrsavla pushes the additional commits
    to the outstanding (still open) PR-111
  • As soon as these commits are pushed, they are visible and usable by
    all (no action required by "main_repo_Admins/committers)
  • @Others https://github.com/others can access/test/comment on the
    updated PR as mentioned above.
  • When websocket_client is tested, complete (ready),
    "Main_Repo_Admin/committer" merges PR-111 into develop. PR-111 is closed.

At this moment there will be the following branches in Sming main repo :

  • Release_1.2 -> Stable release
  • Develop -> Release_1.2 + added websocket_client (which is (close to)
    production ready)

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.

  • Add branch Release_1.3, based on Release_1.2
  • Merge Develop into Release_1.3
  • Developers rebase PR's on Release_1.3

At this moment there will be the following in Sming Main Repo

  • Branch Release_1.2 -> old stable release
  • Branch Release_1.3 -> new (latest) stable release
  • Branch Develop -> Bases on Release_1.3 .) ready for new
    functionality as above
  • Open PR's for unfinished new functionality based on Release_1.3

If we additionally agree that :

  • When development on a PR is finished (ready to merge) that will be
    done within a week.
  • There will be regular Sming releases (once a month ?)

There will be the an environment which has :

Generic :

  • Known and agreed functionality for each release
  • Stable releases
  • Regular releases
  • A Develop release which is close to stable and can be used/tested by
    users with less detailed (internal) knowledge of included changes.
  • The open PR's provide a complete overview of new functionality in
    progress

Users :

  • Know which functionality is available is stable in each release
  • Are free to use current or old release -> upgrade in their projects
    timing
  • Can use develop for reasonable stable new functionality

Developers :

  • Can initiate work/update/finish PR's without intervention from
    "main_repo_admin/committers"
  • Have clear and complete overview of discussions/issues on their PR's
  • Have clear overview of open "functionality in progress" and "Not yet
    complete bugfixes"
  • Have full access to complete and latest source code of new
    functionality without combining several PR's

Main Repo Committers :

  • Will do a merge to develop only when PR is finished
  • Will merge finished PR's to develop within agreed time interval
  • Have no time pressure to merge all updates on open PR's
  • Will do a merge to new release on regular intervals

Generic remarks :

  • The above is a procedure for most of the new functionality.
  • For extra large/complex new functionality a "one time" specific
    process can be created, probably with use of special additional branches.
  • Although less "time pressure" on PR updates, there is still the need
    for more "Main_Repo_Admin/Committers"
  • The steps above are compatible with Travis-CI which, when setup
    properly, will test open PR after each update.
  • The above is close to the process @flexity proposed (use of Branch
    instead of Tag). I stayed at branches as (I think) for the less experienced
    (github) user it is more common to download branches.
  • We should make sure that a "Release Branch" has the correct libsming
    library included. It should be usable without users compiling sming core.
  • I am thinking on an extension to the above by asking
    "Main_Repo_Admins/Committers" to provide a quick initial thought on the PR.
    That would prevent activity/work on PR's which concern/are against Sming
    philosophy.

And as always : Please comment, I will answer and explain any unclearity

Happy reading


Reply to this email directly or view it on GitHub
#230 (comment).

@avr39-ripe
Copy link
Contributor

@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.

@hreintke
Copy link
Contributor

@kireevco :
Thanks for the positive feedback.
But "Feel free to take a lead and implement it!" -> Is not possible without "Main_Repo_Admin/Commiter"

@kireevco : @anakod :
As you know, the both of you are the only one have privileges on the Main Sming repo.
Kireevco on a "long holiday" and Anakod "busy with (too ?) little time to be really active on sming.
What is the idea/progress in the coming time on

  • Managing PR's ?
  • Managing bugfixes ?
  • Implementing rBoot, a major implementation of new functionality
  • Merging to master/other branches ?

@flexiti
Copy link
Contributor

flexiti commented Sep 17, 2015

@hreintke

"When websocket_client is tested, complete (ready), "Main_Repo_Admin/committer" merges PR-111 into develop. PR-111 is closed."

it must be clarified,
how and when to report a readiness to merge eg. a situation that nobody test for two or more weeks because eg. no one has such a Flash, sensor, LCD or ....time :)

otherwise we will have again a lot of "branch" which now will be called the PR-111, etc.

@raburton
Copy link
Member

@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.

@piperpilot
Copy link
Contributor

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!

@hreintke
Copy link
Contributor

@raburton : @piperpilot
I am not that experienced in github so started searching for a way to do what I wanted instead of making use of the (small/partial) knowledge I had.
Found comments like these :

Fortunately, GH creates a remote ref for each PR, so you can use the following syntax to create a local branch that reflects the PR :

git fetch origin refs/pull/PR_NUMBER/head:LOCAL_BRANCH

Elegant and convenient!

For example:

git fetch origin refs/pull/611/head:pull_611
git checkout pull_611

Change /head to /merge to get the ref to the pull request merged with its intended target branch.  

And on the github site, hidden in a help on working on stale pull request.
https://help.github.com/articles/checking-out-pull-requests-locally/

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 😄

@hreintke
Copy link
Contributor

@flexiti :
Yes, I agree that we need a way to decide on readiness/completeness of PR's

  • Who decides
  • Based on what criteria
  • In what timeframe

I started a (little) with that in my generic remarks.

I am thinking on an extension to the above by asking "Main_Repo_Admins/Committers" to provide a quick initial thought on the PR. That would prevent activity/work on PR's which concern/are against Sming philosophy

But definitely needs to be extended.
Probably/Maybe with a different options for "Sming Core" vs "Supplied libraries" as there is more or less dependency on specific hardware.

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

@AutomationD
Copy link
Contributor Author

@hreintke https://github.com/hreintke Unfortunately I don't have any
permissions to repo management. That's why I asked for migrating the repo
to a organization.

On Thu, Sep 17, 2015 at 4:37 PM, hreintke notifications@github.com wrote:

@flexiti https://github.com/flexiti :
Yes, I agree that we need a way to decide on readiness/completeness of PR's

  • Who decides
  • Based on what criteria
  • In what timeframe

I started a (little) with that in my generic remarks.

I am thinking on an extension to the above by asking
"Main_Repo_Admins/Committers" to provide a quick initial thought on the PR.
That would prevent activity/work on PR's which concern/are against Sming
philosophy

But definitely needs to be extended.
Probably/Maybe with a different options for "Sming Core" vs "Supplied
libraries" as there is more or less dependency on specific hardware.

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


Reply to this email directly or view it on GitHub
#230 (comment).

@raburton raburton mentioned this issue Sep 25, 2015
@hreintke
Copy link
Contributor

hreintke commented Oct 2, 2015

@kireevco @flexiti @robotiko @raburton @piperpilot @Others

As promised a new description of my proposal for the workflow of sming.
I adapted my previous proposal to a more standard Github flow.


In "normal" situation there will be the following branches in Sming Main Repo :

  • Main -> Stable release
  • Development -> After release identical to release, will be used for development of next release.

And a Release in Github repository called "Sming_1.2.0"

Process for new functionality to be added :

  • @FunctionaltyProvider writes "New/adapted functionality" in private environment
  • When appropriate (he is finished/want to discuss further progress with others) he initiates a PR against Development Branch (PR numbered 111)
  • The PR should only contain 1 commit (squashed local commits)
  • This PR is not merged into develop but stays open for further progress.
  • @Others can test/use this by : clone main_repo_branch develop, fetch PR-111 (git fetch origin pull/111/head), compile and test.
  • @Others will comment, find issues, propose updates to the PR, all will be communicated within the open PR-111 at Github
  • @FunctionalityProvider adapts the "New/adapted functionality" code according to the remarks
  • @FunctionalityProvider pushes the additional commits to the outstanding (still open) PR-111
  • As soon as these commits are pushed, they are visible and usable by all (no action required by "main_repo_Admins/committers)
  • @Others can access/test/comment on the updated PR as mentioned above.
  • When "New/adapted functionality" is tested, complete (ready), "Main_Repo_Admin/committer" merges PR-111 into branch Development. PR-111 is closed.

At that moment there will be the following branches in Sming main repo :

  • Master -> Stable release
  • Development -> Master (= Release 1.2) + added websocket_client (which is (close to) production ready)

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.

  • Merge Development into Master
  • Create release in Github called "Sming_1.3.0"
  • Open PR's are rebased against "New Master"

At that moment there will be the following in Sming Main Repo

  • Branch Master -> Contains Release 1.3.0
  • Branch Development -> Based on Release_1.3.0, ready for new functionality as above
  • Open PR's for unfinished new functionality based on Release_1.3.0
  • Two Releases in Github directory -> "Sming_1.2.0" and "Sming_1.3.0"

Process for Bugfixes :
There will be two types of Bugfixes

  • Non Urgent fixes which can wait until next release
    These will be handled using the "new functionality" process -> incorporated in next release
  • Urgent fixes which have to be applied asap (hotfix).
    A (squashed) PR against Master is submitted.
    This PR is tested in the same way as "new functionality" only using Master as base to test against.
    When testing is completed. The PR is merged in both Master and Development (eventually fixing compatiblity issues with Development)
    Decision is made whether "non urgent bugfixes" will also be included -> Merged to Master also.
    A Release is created in Github called "Release_1.3.1"

At that moment there will be the following in Sming Main Repo

  • Branch Master -> Contains Release 1.3.1
  • Branch Development -> Based on Release_1.3.1, including finished functionality for "Sming_1.4.0"
  • Open PR's for unfinished new functionality based on Release_1.3_1
  • Three Releases in Github directory -> "Sming_1.2.0", "Sming_1.3.0" and "Sming_1.3.1"

Introduction of "Large Impact" functionality
This process which should only be used when necessary. Expection is very limited usage.
It can/will be adapted when specific requirements are needed.

For these kind of PR's in general the following procedure is taken.

  • @FeatureProvider writes "Feature functionality" in private environment
  • When appropriate (he is finished/want to discuss further progress with others) he request a "Featurte Implementation"
  • A new branch "Feature/Desription" is created, based on Master
  • A (squased) PR is submitted against "Feature/Description"
  • An issue "Feature/Description/Comments" is created in SmingHub/Sming.
  • @FeatureProvidor gets write allowance to SmingHub/Sming -> Restriction only to update branch "Feature/Desription"
  • @Others will comment, find issues, propose updates to the PR, all will be communicated within the open "Feature/Description/Comments" at Github
  • @FeatureProvidor adapts the "Feature functionality" code according to the remarks
  • @FeatureProvidor pushes the additional commits against branch "Feature/Description" and merges these.
  • @Others can submit PR's against "Feature/Description"
  • @featureProvidor" evaluates these PR's and when accepted merges these in "Feature/Description"
  • As soon as these commits are pushed/merged, they are visible and usable by all (no action required by "main_repo_Admins/committers)
  • @Others can access/test/comment on the updated Feature by pull if the "Feature/Description" branch which is always up to the last status.
  • When "Feature functionality" is tested, complete (ready), "Main_Repo_Admin/committer" merges branch "Feature/Description" into Development.
  • Branch "Feature/Description" is closed.

Additionally agree that :

  • When development on a PR is finished (ready to merge) that will be done within a week.
  • There will be regular Sming releases (once a month ?)

Next steps
If we agree with the proceduere above the following steps should be taken.

  • Create release "Sming_1.3.0" from current Master
  • Create new branch "Development" based on Master
  • Identify Bugfixes/functionality PR's already included into Develop
    Decide on status of those PR's
    Finished/accepeted ones -> Merge into "Development"
    Unfinished PR's re-submitted against Development, finish following new procedure.
  • Identify open "feature/functionalty" branches currently open
    Decide on which process they will follow to complete
    "New/adapted functionality" -> re-submit against Development
    "Feature" ones -> rebase against Master, follow new procedure
  • Document this process on Sming Wiki
  • Create a Wiki pages which describes functionality of available releases
  • Update Main Wiki page to include latest release information.

Please comment

@piperpilot
Copy link
Contributor

All of this sounds pretty sensible to me. I think we need to document 2 separate things...

  1. What the process from a maintainer is
  2. A simplified version of the process for a contributor that just wants to get a simple fix/feature in

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.

@AutomationD
Copy link
Contributor Author

@hreintke I fully trust you!

@jedahan
Copy link

jedahan commented Oct 2, 2015

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

  • Fork master to a new feature branch
  • Develop locally
  • Make a pull request with the local commits squashed
  • Maintainers and testers test the change, and give feedback until they are comfortable giving 👍
  • After n 👍s, merge into master
  • Release new version, using semver or ferver to reduce overhead

Upsides

  • easier to contribute
  • less maintenance for project maintainers
  • encourages testing of code before merging

Downside

  • developing against multiple unmerged pull requests is trickier

@hreintke
Copy link
Contributor

@kireevco : Can we close this issue ?

@AutomationD
Copy link
Contributor Author

AutomationD commented Nov 29, 2015 via email

@hreintke
Copy link
Contributor

A process for contributions is agreed.
See https://github.com/SmingHub/Sming/blob/develop/CONTRIBUTING.md for process.

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

No branches or pull requests

10 participants