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

Release Schedule #341

Open
jisaacks opened this issue Nov 22, 2016 · 31 comments
Open

Release Schedule #341

jisaacks opened this issue Nov 22, 2016 · 31 comments

Comments

@jisaacks
Copy link
Owner

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

@r-stein
Copy link
Collaborator

r-stein commented Nov 22, 2016

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.

@rchl
Copy link
Collaborator

rchl commented Nov 22, 2016

I'm all up for trying it.

@jisaacks
Copy link
Owner Author

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.

@rchl
Copy link
Collaborator

rchl commented Nov 22, 2016

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

@jisaacks
Copy link
Owner Author

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
Copy link
Owner Author

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.

@rchl
Copy link
Collaborator

rchl commented Nov 22, 2016

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

@jisaacks
Copy link
Owner Author

@rchl that makes sense.

@deathaxe
Copy link
Collaborator

deathaxe commented Nov 22, 2016

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
Copy link
Collaborator

deathaxe commented Nov 28, 2016

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.

@rchl
Copy link
Collaborator

rchl commented Dec 4, 2016

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

@deathaxe
Copy link
Collaborator

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:

@deathaxe
Copy link
Collaborator

deathaxe commented Feb 2, 2017

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

@FichteFoll
Copy link

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

@deathaxe
Copy link
Collaborator

deathaxe commented Feb 3, 2017

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
Copy link
Collaborator

deathaxe commented Feb 3, 2017

@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 Fix: Run all git commands from within working tree #365
  2. Fix for annoying issue Noticeable flicker/delay in icons when working with files #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 Problem with smudge/clean filters #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.

@r-stein
Copy link
Collaborator

r-stein commented Feb 22, 2017

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.

@jisaacks
Copy link
Owner Author

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

@deathaxe
Copy link
Collaborator

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

@r-stein
Copy link
Collaborator

r-stein commented Feb 23, 2017

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

@deathaxe
Copy link
Collaborator

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.

@jisaacks
Copy link
Owner Author

jisaacks commented Feb 24, 2017

@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
Copy link
Owner Author

@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
Copy link
Owner Author

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

@deathaxe
Copy link
Collaborator

deathaxe commented Feb 25, 2017

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 Do some final code cleaning before release. #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
Copy link
Collaborator

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.

@FichteFoll
Copy link

FichteFoll commented Feb 26, 2017

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: wbond/packagecontrol.io#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: #359 (comment)

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.

@deathaxe
Copy link
Collaborator

deathaxe commented Feb 26, 2017

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
Copy link
Collaborator

@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
Copy link
Collaborator

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

@jisaacks
Copy link
Owner Author

Sounds good to me.

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

No branches or pull requests

5 participants