GitGutter icon indicating copy to clipboard operation
GitGutter copied to clipboard

Release Schedule

Open jisaacks opened this issue 7 years ago • 31 comments

@r-stein @rchl

Hey guys, I have been really appreciating the all the hard work you have been doing. In light of all the recent activity. I wanted to discuss on a plan for managing everything.

What do you think of always having an open issue for the next release, where we can keep track of what is going to be included in the next release, the changelog etc. It would be a central communication hub for everyone to communicate on progress for next releease. Then once everyone approves the release. I can cut it. I would then be fine with y'all merging PRs as you see fit, as long as we are adding to the changelog and all in communication.

This plan may suck, so let me know what y'all think or any modifications to this plan, of if you just think it's bad idea, etc. Let me know! Thanks.

jisaacks avatar Nov 22 '16 17:11 jisaacks

Using milestones seems to be a good idea to group PRs and issues for releases. Additionally creating an issue per release isn't bad either.

r-stein avatar Nov 22 '16 17:11 r-stein

I'm all up for trying it.

rchl avatar Nov 22 '16 17:11 rchl

Do y'all have merge rights?

Let's try this, if one of you approves, the other feel free to merge (prefer rebase.) That way you are always both signing off. Let's try that and see how that works.

And lets try to add milestones, and an issue tracking each release if we can. I'll still cut the releases.

jisaacks avatar Nov 22 '16 17:11 jisaacks

I think I do. At least I see the merge button.

As for "prefer rebase". I hate including fixup! commits or commits that just fix issues in the pull request in the final history. So would prefer squashing all commits (or at least do it when it makes sense).

#331 is one example where I would prefer squashing as it has some commits that fix earlier problem of the pull request or just adds changes that are later removed.

But I guess common sense should be applied on a case basis...

rchl avatar Nov 22 '16 17:11 rchl

So would prefer squashing all commits (or at least do it when it makes sense).

My only reasoning against squashing, is I like to give contributors, higher numbers in the commit graph if they did a lot of work. If someone wrote 50 commits, and we squash it to 1 commit, it puts them low on the commit graph, when in fact they helped out more than what the graph portrays.

#331 is one example where I would prefer squashing as it has some commits that fix earlier problem of the pull request or just adds changes that are later removed.

If it makes sense to squash it or merge, I am fine. I just prefer rebasing if possible because it usually makes the tree a little cleaner. But isn't always worth it.

Just use your own judgement/discretion.

But I guess common sense should be applied on a case basis...

Exactly! :)

jisaacks avatar Nov 22 '16 17:11 jisaacks

Oh one last caveat, if someone wants to make a change that fundamentally changes gitgutter, or is controversial, etc. I'd like to be included in the decision before merging. Otherwise for bug fixes, small features, etc. Feel free as long as any other member approves first.

jisaacks avatar Nov 22 '16 17:11 jisaacks

My only reasoning against squashing, is I like to give contributors, higher numbers in the commit graph if they did a lot of work. If someone wrote 50 commits, and we squash it to 1 commit, it puts them low on the commit graph, when in fact they helped out more than what the graph portrays.

I would hope that such trivial thing as stats wouldn't contribute to degradation of master branch history. :) It's much easier to blame the code, revert changes or just read the branch history if it's all nice and tidy. Also it's beneficial when creating changelogs. And creating a lot of commits should be discouraged in favor of creating more, smaller pull requests. :)

If it makes sense to squash it or merge, I am fine. I just prefer rebasing if possible because it usually makes the tree a little cleaner. But isn't always worth it.

Squashing implies rebasing I believe. So it has the clean look of rebased branch + clean branch history.

All in all, that's just my opinion and what I would apply to my project. I'm not gonna force anything in here, just stating my way of doing things. :)

rchl avatar Nov 22 '16 18:11 rchl

@rchl that makes sense.

jisaacks avatar Nov 22 '16 18:11 jisaacks

I totally agree with @rchl about squashing. I also like clean history in the end, so you can clearly see a relationship between issues and the resulting changes. Smaller fixes like in #331 in code style or commits which were created during a developement discussion don't need to be kept after merging.

deathaxe avatar Nov 22 '16 20:11 deathaxe

I am quite far with my solution for Issue #26 which also brings along significant lower latencies to add the gutter icons especially for large files.

The work is mainly based on PR #338 which could maybe solve Issue #279 even though I am not quite sure with that. This issue might also be caused by empty output of git or an exception in update_git_file() which would be caught silently at the moment. With PR #344 we would at least get an idea of it, as some recommended exception handlings would print issues with git execution in this situation to console.

So can we check PR #338 and PR #344 and merge it, so I can prepare to propose the performance update?

PR #343 is splitted into several commits as it introduces some bigger changes I found useful to to act GitGutter somewhat smarter and clean up some cross references in the python modules. I guess a review of those changes takes a bit longer, so it is not so importand to have this merged, too.

But as the show_diff modue will change nearly completely to achieve the performance boosts it would make things easier, too. Otherwise I would need to resolve some conflicts with it later.

deathaxe avatar Nov 28 '16 19:11 deathaxe

@deathaxe Sorry, but I've reverted a bunch of your latest fixes from master as they didn't have review. As said in this thread, all changes should be reviewed by at least one person.

And also those commits broke Mac and Linux + ST2 (I believe, looks like that from the code at least). Please resubmit those as pull requests and don't merge until reviewed.

rchl avatar Dec 04 '16 13:12 rchl

Can you check PR #351, #354, #355, #356 for remaining issues, please?

I am nearly complete with preparing a PR to fix the following issues on top of those commits:

  • Issue #226: Show file status (commited, untracked, ignored) in taskbar by replacing git ls-files by git status
  • Issue #233: git diff will no longer be called for commited, untracked, ignored files/views
  • Issue #147 / #349: Heavily reduced delays and removed flickering

deathaxe avatar Dec 17 '16 09:12 deathaxe

Release strategy

@jisaacks: Continued discussion from #359

When calling the git: generate changelog it asks for the tag to compare against with the most recent one as default value. The changelog contains all short messages of all commits younger then the selected tag. If the short message is formated correctly and contain useful information the changelog is ready. In worst case you might select a couple of commits not worth mensioning in the changelog, what might be easier than look through the commits and pick commit messages manually.

As we will need to add pre-release tags like 1.5.0-pre1 or 1.5.0-rc1 GitSavvy will suggest the latest pre-release as base for changelog creation. I would prefere to add release messages to final releases only containing everything since the last real release. Pre-release tags may be simple tags only, while final releases use full annotated tags?

Tagging suggestion

1.5.0-pre1 - tag with maybe new features 1.5.0-rc1 - feature freeze for bug fixing only

deathaxe avatar Feb 02 '17 18:02 deathaxe

Please use dots between numeric and non-numeirc identifiers, or you might be in for a surprise. SemVer explictly does not use "natural sort".

(Always triggers me when reading this in a semver-compliant situation.)

FichteFoll avatar Feb 03 '17 01:02 FichteFoll

You mean:

1.5.0-pre.1 - tag with maybe new features 1.5.0-rc.1 - feature freeze for bug fixing only

Thanks for the tip!

deathaxe avatar Feb 03 '17 17:02 deathaxe

@jisaacks: As GitGutter-Edge is already removed from package control repositories we may

  1. decide how to tag pre-releases
  2. mark the master as pre-release on some good state
  3. publish the new strategy and the way how to install pre-releases to people out there, who are interested in testing latest versions.

I guess many people may wonder where and why GitGutter-Edge is gone.

We at least need to publish the information to add the following key to Package Control.sublime-settings to be the replacement for GitGutter-Edge.

	"install_prereleases":
	[
		"GitGutter"
	],

There are some local commits pending I would like to merge to master and ask to cut of the next release then:

  1. Fix for work-tree support PR #365
  2. Fix for annoying issue #340 which needs (1).
  3. Enhancement to resize the minimap marker or highlight the whole line, which requires (2). This changes few lines introduced by (2), but I want to keep it seperate.
  4. Fix for issue #74, which is a very small fix in the end to make GitGutter respect smudge filter settings correctly.

I am not sure how long it takes to review all these changes so we might get ready for the next release not too far in the future.

deathaxe avatar Feb 03 '17 17:02 deathaxe

I think we should merge #372 and issue a release soon afterwards (maybe a prerelease a few days before). This fixes the flickering, which may annoy a lot of users.

r-stein avatar Feb 22 '17 20:02 r-stein

@r-stein sounds good. @deathaxe if you want to generate the change log using your tool I can cut a release when y'all are ready.

jisaacks avatar Feb 23 '17 15:02 jisaacks

Ok, will rebase the PR branches, autosquash the fixups and generate a PR with the changelog.

deathaxe avatar Feb 23 '17 16:02 deathaxe

I will create a small PR to improve the readability of the popup css since we now encourage people to change it.

r-stein avatar Feb 23 '17 18:02 r-stein

Thank you very much for patiently testing my changes! Cool to get these results publishedm soon.

Maybe this is a good time to think about what could come after 1.5.0.

Here are some ideas:

  1. Using git status --porcelain=2 to read the current branch, commit hash and file status (staged,untracked,ignored) instead of using several git calls to receive these information could help reducing latencies and reduce number of program calls.
  2. The information from 1. could help to correctly check for staged files and implement "Compare against Staging Area" functionality.
  3. The settings module needs some attention, too, as it does not support project or view based settings very well.
  4. I would like to move more internal functions the 'modules' directory in order to hide them from ST.
  5. The GitGutterHandler class got quite huge and may need to be splitted into smart pieces somehow. (Not yet sure how and where to split, right now)
  6. Maybe it is a good idea to flatten the promise chains a bit. I found a good tutorial how to do it. This could help splitting GitGutterHandler and make it easier to understand how the chain works.

deathaxe avatar Feb 23 '17 18:02 deathaxe

@deathaxe I see you added a tag for 1.5.0

A couple things.

I see you added a message for 1.5.0 in release_messages/src This is good. But it's actually the release_messages/dest that package control uses to generate the releases. Nothing in the dest folder should be touched manually though. It gets generated from the src by running make build_release. See the makefile.

Currently it gets set as a prelease based on wheather or not the file name contains the text pre which currently you have it named as 1.5.0.txt. So we will need to rename it to pre or I can modify the build.rb file to determine if it is a pre-release by some other means? I am fine if we want to use some other indicator of whether it is a pre release.

No worries, though, and thanks for spear heading all this.

But to fix it, I think you need to rename /release_messages/src/messages/1.5.0.txt to something like /release_messages/src/messages/1.5.0-pre.txt, then run make build_release to actually build the release notes. Then commit those changes. Then change the tag to point to the new commit.

Then I can run make release which will actually send the release to package control.

Let me know if you have any questions. Thanks! :)

jisaacks avatar Feb 24 '17 22:02 jisaacks

@deathaxe, Also you don't need to add the Changes since 1.4.0: to the message. Package control merges all the release notes base on what version to upgraded to and from, so you are always looking at everything that is new, and only what is new.

jisaacks avatar Feb 24 '17 22:02 jisaacks

@deathaxe actually I think I mis spoke. Don't add the tag. That is what my make release script does. Hmm. Actually by adding that tag, I think you did cut a release. But there will be no release messages when people update because the dest release messages were not generated. That's the reason I wanted to be responsible for cutting the releases.

So yeah, not really sure what we should do now since it's likely that everyone will be updated, just without any release notes.

jisaacks avatar Feb 24 '17 22:02 jisaacks

release message

I see you added a message for 1.5.0 in release_messages/src This is good. But it's actually the release_messages/dest that package control uses to generate the releases.

The basic structure how your scripts work is clear to me.

I basically intended to provide you the changelog and leave creating the 'dest' files up to you on final release of 1.5.0. I was not aware of all release messages being merged as each message of each package looks differnt, I didn't simply realize it. This was the question in PR #376 how to handle it. I was afraid of release messages added to pre-releases would not be displayed to users not installing pre-releases. That's why I asked, whether it is sufficient to add them to pre-releases or releases only. So the answer is: Add to pre-releases, too.

next steps

So yeah, not really sure what we should do now since it's likely that everyone will be updated, just without any release notes.

This shouldn't have been a final release but one, which is installed to users, who have GitGutter added to the list of "install_prereleases". This was what I understood Package Control to work like, if I add the PRERELEASE tag. So I obviously missed something. Sorry.

  1. You could use my release message 1.5.0.txt from 'src' to create a release candidate 2 with the release message in 'dest' being added? Just rename and change it as you need. It's more efficient if you do I think (I have no ruby). I can than have a look on it and learn.

  2. I fixed some minor linting errors yesterday and created PR #378 . Maybe you can have a look for some regressions and if it is ok, merge and include it into 1.5.0-rc.2

pre-release naming

Several packages use alpha,beta,rc to mark the several stages of pre-releases instead of pre only. I am basically fine with all kinds of naming. I indirectly asked for the naming strategy some posts above:

1.5.0-pre.1 - tag with maybe new features 1.5.0-rc.1 - feature freeze for bug fixing only

Too indirect, maybe :-)

deathaxe avatar Feb 25 '17 13:02 deathaxe

I am a bit confused about the packagecontrol.io stats.

With GitGutter not in the list of install_prereleases, Package Control installs 1.4.0 as expected. With GitGutter in that list Package Control lists and upgrades to 1.5.0-rc.1 as expected, too.

The stats show 0 installs today and only few yesterday.

Does this mean everybody in the world has GitGutter included in the list of install_prereleases which are not displayed in the stats of 1.4.0?

Why is the 1.5.0-rc.1 not displayed as PRE like the PackageDev package? Is it because of the PRERELEASE tag on GitHub? I somehow miss something about pre-release handling in Package Control.

deathaxe avatar Feb 25 '17 14:02 deathaxe

The pre-release tag is picked up in https://packagecontrol.io/channel_v3.json. It's probably just the packagecontrol.io package info page that fails to render the pre-release for some reason. The release auto-tagging feature is not as mature as you might expect. I have reported a similar issue with PackageDev: https://github.com/wbond/packagecontrol.io/issues/93.

This was what I understood Package Control to work like, if I add the PRERELEASE tag.

No, Package Control only operates on tag names, not on "github release flags" or something. I also mentioned this here: https://github.com/jisaacks/GitGutter/pull/359#issuecomment-276697472

Stats are looking fine. There appear to be less installations on the weekend and the current numbers seem to be consistent. Furthermore, I don't think that updates count as installations, but I never worked on any stat sending-related code.

If you have any more questions, feel free to ask.

FichteFoll avatar Feb 26 '17 14:02 FichteFoll

With the merge of PR #378 I am done with preparations for a final 1.5.0 release.

Now it's up to @jisaacks when and how to make the final cut including the message in the 'dest' folder ;-)

EDIT:

You should include an update message in your next release indicating that previous GitGutter-Edge users should add GitGutter to their install_prereleases setting array in PC's setting file.

... as @FichteFoll suggests.

deathaxe avatar Feb 26 '17 14:02 deathaxe

@FichteFoll : This was the first time I got in touch with that RELEASE / PRERELEASE thing on GitHub. I previously didn't care about it at all - using only tags. So I had to learn the difference between the PRERELEASE flag and pre-tag by appending any -rc, -pre etc. Thanks for updating me.

deathaxe avatar Feb 26 '17 14:02 deathaxe

@jisaacks and @r-stein : We should maybe release a V1.5.1 to fix the Issue #381 for common users? I guess many will struggle with it.

deathaxe avatar Mar 14 '17 17:03 deathaxe