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

New strategy for development + deploying code #321

Closed
miketaylr opened this issue Nov 4, 2014 · 32 comments
Closed

New strategy for development + deploying code #321

miketaylr opened this issue Nov 4, 2014 · 32 comments

Comments

@miketaylr
Copy link
Member

Could probably pick every single label...

@karlcow has pointed out that the way that we're doing things is parts confusing, weird, and broken. And I'm afraid it's contributor hostile. So let's try to fix it.

How we do things now:

  1. deploy to webcompat.com from master. This means master is frozen until we have a feature set ready to deploy. This has historically been just a few commits, or recently weeks worth of work.
  2. have dev.webcompat.com branch for developing new stuff on. And deploying to staging.webcompat.com from that. Unfortunately Karl points out that stuff gets "stuck" here, waiting for other unrelated things. Like documentation or other features.

So where do we go from here? Here's a proposal. This is wide open, so please add suggestions or proposal alternatives.

  1. Have a master branch that's always working. We pull in from pull requests or merge in feature branches. More on working in smaller bite-sized issues later. But anything that gets merged into master must be standalone and deployable. We deploy from master to production and create a version tag v0.1, etc. So if we need to roll back, we deploy the previous version tag.

  2. Have a staging branch that we merge master into when we want to test things on staging.webcompat.com. Maybe this branch isn't needed. I don't know.

  3. We kill dev.webcompat.com branch.
    (and then of course we document this).

Maybe that's describing http://nvie.com/posts/a-successful-git-branching-model/ without the development branch?

@karlcow
Copy link
Member

karlcow commented Nov 4, 2014

First. BIG THANKS for opening the issue. ❤️

As I said in an email, my basic requirements are:

  1. No big intertwined releases (aka code stalled)
  2. Well documented step by step on how to process and participate
  3. Maximizing the efforts for helping people to get into it

@karlcow
Copy link
Member

karlcow commented Nov 4, 2014

Because Mike mentioned it on IRC today.
http://paulhammant.com/2014/01/08/googles-vs-facebooks-trunk-based-development/

@magsout
Copy link
Member

magsout commented Nov 4, 2014

  1. Add changelog
  2. roadmap ?

@miketaylr

  1. Have a master branch that's always working. We pull in from pull requests or merge in feature branches.

Each features need a branch ? We commit in it, and merge into master when it finished ?
We don't commit in master ? Just use PR ?

@MoOx
Copy link

MoOx commented Nov 4, 2014

@magsout ask me to bring some experience here, so here are some ideas (I don't have any experience with webcompat website so not sure if everything is relevant).

Using something like Travis-CI, you could go for a simple workflow (if you test coverage is high):

  • master is always good, tested & deployed automatically (when tests are ok) on webcompat.com. If it's a static website, you can easily use gh-pages with (grunt|gulp)-gh-pages. This should be easy to setup.
  • PR are tested & thanks to github/travis integration you will see from github interface if pr is breaking things or not (a PR without any test should not be merged). This is very easy to setup.

Every changes pass by a PR, so you see if tests are ok or not.

I think staging.webcompat.com should still be handled by hand, for big occassion only.

@karlcow
Copy link
Member

karlcow commented Nov 4, 2014

@magsout

Just use PR ?

Yes. This is one of the things I think we should do. It's a lot easier to review and comment on them. For example, I have commit rights on the webcompat repo but I always send PR, specifically for the purpose of the review process.

@karlcow
Copy link
Member

karlcow commented Nov 4, 2014

So looking at Mike's proposals.

  1. Have a master branch that's always working.

Yes. Master is what is deployed whatever the way it is managed and evolved. But basically what is on master is production ready all the time.

We deploy from master to production and create a version tag v0.1, etc. So if we need to roll back, we deploy the previous version tag.

Here I don't understand what @miketaylr is trying to explain. You will have to lead me. 👣

  1. We kill dev.webcompat.com branch.

So for this. I don't know. At least it should be named staging. Or we kill it if we start fresh.

I would love @tagawa opinions too, because I have the feeling we had the same confusion at the beginning of October. Or maybe it's just me.

So in non technical terms, the way I could see it (maybe too complicated?):

  1. We have an idea, we notice a bug, we think about improving a part of the code. We open an issue (we are usually good on this).
    1. If the issue has a very wide scope, subdivide it in smaller atomic issues which can be easily addressed. It will increase the possibility of participation. It will be easier to comment.
    2. Explain what you want to code as it goes. @calexity and @magsout are very good at that usually.
    3. If the code is changing, if you want to revert something and you struggled on an issue, say it in the comment. It's usually cool, because it helps understand what is on your mind. And it helps being part of the project.
    4. If an issue appears during this work, open a new issue. Do not create a commit unrelated to the issue. I know it's tempting. But separation of concerns help on a long term.
  2. On your own copy of the project (personal fork of master), update to the latest version and create a branch for the issue you are working on. Smaller commits might help to understand (depends on your style and many other things). Reference the issue number in your commits. Good for tracking.
  3. Make a pull request with the issue number. It doesn't have to be complete, because additional commits will pile up in the PR and it can be sometimes a good way to have a code review.
  4. Ready and tested (We are not good yet for tests)
  5. We deployed (to define how and when)

I don't have enough knowledge on the system of tags, etc. but whatever we decide it should be documented step by step and we are all working with the same process. Maybe there are things which are easier than others.

What I want to avoid:

Things stucked somewhere (be branch, tags), waiting to be deployed in production because we are working on something which might take longer to sweat out.

An example the rate limiting code for denying further requests is useful for the search and display issue but is unrelated in terms of code. We could imagine deploying the search issue without it (we would shoot in our feet, but it would work). Or we could deploy the rate limiting code before the search view without having deployed the search view. Even further the code blocking a request and the message displayed to the client are also unrelated or let's say weak dependencies.

If we manage to think in this way, it might be a lot easier for us to develop things without the fear of breaking everything. It might constraint a bit more the dev, but it helps by providing flexible articulations in between features and so our own individual codes. I hope I make sense. I can try to explain again.

@miketaylr
Copy link
Member Author

Thanks @MoOx for your suggestions.

Every changes pass by a PR, so you see if tests are ok or not.

Yes, we need to get here. @vladikoff has already bugged me about setting up Travis for the (few) tests that we do have.

I think staging.webcompat.com should still be handled by hand, for big occassion only.

I agree. Scary or big changes that perhaps require sharing with others to test out or give feedback on quickly.

@miketaylr
Copy link
Member Author

Add changelog
roadmap ?

+1 @magsout.

@MoOx
Copy link

MoOx commented Nov 4, 2014

I've planned to publish a post for setup travis - from simple tests to deploy a build (but in french). I'm sure @magsout will translate you the important part :D

@miketaylr
Copy link
Member Author

Merci @MoOx. Je peux comprendre 10 mots en français, so I might be OK. (unfortunately that was like 5 of them :P)

@miketaylr
Copy link
Member Author

We deploy from master to production and create a version tag v0.1, etc. So if we need to roll back, we deploy the previous version tag.
Here I don't understand what @miketaylr is trying to explain. You will have to lead me.

Tagging in git is just a way to create a bookmark to a certain point in history and give it a name. Just saying, hey hash foo was important. http://git-scm.com/book/en/v2/Git-Basics-Tagging is a helpful link. If we have a changelog, it makes sense to version releases. Either by date or semver, I don't really care. Just an identifier to show progress, and something that we can go back to in the event of disaster.

As we deploy from master to our servers, we create a tag that corresponds to whatever heading we add to the changelog. If the sky falls down as a result, we checkout the last known good commit/deploy (which would be the last tag) and deploy that. (Then clean up the mess. 💩).

Another strategy that is used by people is to create a branch from master as a release branch. But I find that gets sort of messy since I have my own feature branches--I'd rather not look at release branches.

I would love @tagawa opinions too

Same. He's always got good advice.

So in non technical terms, the way I could see it (maybe too complicated?):

I think everything here is good. Opening pull requests for all changesets will require a few things:

  1. Me to stop being lazy. There are tools to create PRs from the commandline. This will help. (time to learn https://hub.github.com/ or similar)

  2. A process for code review. Do we require review for everything? Are we willing to review other people's code? What is considered a timely review? Can we merge our own PRs?

A few more notes:

If the issue has a very wide scope, subdivide it in smaller atomic issues which can be easily addressed. It will increase the possibility of participation. It will be easier to comment.

Coming up with a roadmap (on the repo wiki, maybe?) will help with this, I think.

@magsout
Copy link
Member

magsout commented Nov 4, 2014

On your own copy of the project (personal fork of master), update to the latest version

@karlcow why personnal fork? We can create branch into repo of webcompat and Make PR between it and master.

@magsout
Copy link
Member

magsout commented Nov 4, 2014

A changelog is a good way to have a written (in addition to the history of commits) to see the progress of the project history.

A roadmap, provides a visibility on the future, it allows potential contributors working on the project by providing assistance on a particular option.

It seems that everyone agrees on the master branch and add new features thanks to PR.

However, I think we can always use commit for minor fixes

I wonder if the use of the tag is relevant? New features will be added by PR, history will be clear to rollback if necessary. Also, I think the tag is a good things for lib.

Travis-CI @MoOx as suggested is a good idea to keep the integrity of the project. To see if we can use it for deployment.

I think the proposed @karlcow workflow is interesting and relevant except 3) that I raised previously.

@MoOx
Copy link

MoOx commented Nov 4, 2014

IMO changelog work with release/tags. So no tags ? no changelog needed :)
For CI, changelog is commit logs. Especially if you don't commit too much (via PR).

@miketaylr
Copy link
Member Author

On your own copy of the project (personal fork of master), update to the latest version

@karlcow why personnal fork? We can create branch into repo of webcompat and Make PR between it and master.

I would agree this seems like extra work for little benefit (for me personally)--after just having done the exercise. Non repo collabs will have to fork it no matter what, repo collabs can work from their forks or send PRs from webcompat/webcompat.com.

@karlcow
Copy link
Member

karlcow commented Nov 4, 2014

I've planned to publish a post for setup travis

Je peux le traduire aussi. Merci. :)

@karlcow
Copy link
Member

karlcow commented Nov 4, 2014

@karlcow why personnal fork? We can create branch into repo of webcompat and Make PR between it and master.

  • To put any participants on the same foot.
  • To preserve the notion of protected territory for what goes into the project.
  • To avoid branch names clashing
  • Because I'm a heavy branch maker, see my current list
  0a
  credentials
  docs
  domaindetection
  issue273-githubrequest
  janitor
  labelescaping
  logging
* master
  rate
  tests
  ucweb
  urlInTitle
  0a
  credentials
  docs
  domaindetection
  issue273-githubrequest
  janitor
  labelescaping
  logging
* master
  rate
  tests
  ucweb
  urlInTitle
  remotes/origin/HEAD -> origin/master
  remotes/origin/aboutpage
  remotes/origin/credentials
  remotes/origin/dev.webcompat.com
  remotes/origin/docs
  remotes/origin/domaindetection
  remotes/origin/formstyle
  remotes/origin/homepage-issues
  remotes/origin/homepage2
  remotes/origin/issue273-githubrequest
  remotes/origin/janitor
  remotes/origin/master
  remotes/origin/rate
  remotes/origin/tests
  remotes/origin/urlInTitle
  remotes/origin/yellowstyle

Imagine 5 me and the list becomes very long a lot less manageable.

@miketaylr
Copy link
Member Author

That makes sense. Personally I delete branches after the work has been merged.

@magsout
Copy link
Member

magsout commented Nov 5, 2014

The advantage of working on a branch of webcompat is to work several. propose improvements. work someone with a fork is a bit more complicated.

@karlcow
Copy link
Member

karlcow commented Nov 5, 2014

@magsout

Do you mean "working with someone on a fork is more complicated?"
And you prefer "working on a branch of webcompat so more than one person can commit on the same branch" ?

I don't mind working (even can be very cool) on the same branch with someone but only and only if the branch are really atomic modifications and are here to solve one issue. In this case, we could even adopt conventions for branch naming such as git checkout -b issue_issuenumber or anything that would make sense.

On the other hand, one problem I can see with it is that we will enter into a dance of MERGE conflict. Two persons working offline (local) on the same branch.

hmm… choices. 💬

@magsout
Copy link
Member

magsout commented Nov 5, 2014

@karlcow

Do you mean "working with someone on a fork is more complicated?"

Yes ;)

The advantage of working on branch and make PR between this branch and master:

  1. able to commit (for small changes, a class error etc ..)
  2. to make a PR of PR easier.

Work from fork complicates these examples.

@magsout
Copy link
Member

magsout commented Nov 5, 2014

@MoOx

IMO changelog work with release/tags. So no tags ? no changelog needed :)

Why ? Changelog is cool, just specify big features (new api, new page etc).., to have an history more user friendly than commit, no ?

@MoOx
Copy link

MoOx commented Nov 5, 2014

Not sure how will read a changelog for a website. But why not :)

@karlcow
Copy link
Member

karlcow commented Nov 5, 2014

@MoOx webcompat.com is indeed a Web site, but I think @magsout is talking about the development changelog. This is a webapp for managing reports on Web compatibility across browsers. So the changelog is more for devs than for the user of the site.

@miketaylr
Copy link
Member Author

I think it should be fine for collabs to work in branches from the main repo. Also fine for collabs and non-collabs to work in their own forks. I would propose that we delete branches from the main repo as the work is merged in to keep things clean. So if that makes you uncomfortable, working on your own fork (where we can't touch your branches) is the way to go.

So we have a living, stable master. Let's try to tag releases and come up with a changelog for the repo. Let's make a staging branch.

Let's document all of this, and give a warning that dev.webcompat.com branch will be deleted in the near future, so people can migrate any work from there). That's stuff we can start working on today.

Continuous integration and deployment (+ better test support) to come.

Any objections?

@magsout
Copy link
Member

magsout commented Nov 5, 2014

LGTM

@tagawa
Copy link
Member

tagawa commented Nov 6, 2014

Sorry, I'm not an expert on development best practice, but a few comments:

  • I agree that one problem we're trying to fix is having a code release blocked because we're waiting on some other (unrelated) code. Whatever fixes this is good.
  • I'd like to avoid situations where I submit a PR from origin and find other developers' commits appear in the upstream PR.
  • Personally I like personal forks because they feel safe (and I'm scared of merge conflicts).
  • I like the idea of having a non-public server to check before release, but having both staging and dev seems to be overkill.
  • Changelog is a nice-to-have but not essential.
  • Tagging and being able to rollback is a good idea.

So just so I understand, the current proposal is to have PRs on branches sent directly to master? And staging is only used for larger chunks of work?

@magsout
Copy link
Member

magsout commented Nov 6, 2014

@tagawa

So just so I understand, the current proposal is to have PRs on branches sent directly to master? And staging is only used for larger chunks of work?

yes exactly .

@miketaylr
Copy link
Member Author

I'd like to avoid situations where I submit a PR from origin and find other developers' commits appear in the upstream PR.

If we're all just using master (so a dev branch doesn't get out of sync ) this shouldn't be a problem. That said, get used to rebasing against master before sending a PR. Otherwise there are no guarantees.

I like the idea of having a non-public server to check before release, but having both staging and dev seems to be overkill.

Yeah, dev is going to disappear. Staging will probably only be used if someone asks, hey can you deploy this branch to staging I want to test something or share it with someone.

@miketaylr
Copy link
Member Author

OK, it sounds like most everyone is on board. Let's give it a shot and revisit it if there are pain points.

@miketaylr
Copy link
Member Author

Accidentally merged my own PR. Yay for growing pains.

Here's the updated submission guidelines if anyone would like to review or suggest tweaks: https://github.com/webcompat/webcompat.com/blob/master/CONTRIBUTING.md#submission-guidelines

@miketaylr
Copy link
Member Author

I think we're done. Yay.

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

5 participants