-
Notifications
You must be signed in to change notification settings - Fork 223
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
Comments
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. |
I'm all up for trying it. |
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. |
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... |
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.
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.
Exactly! :) |
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. |
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.
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 that makes sense. |
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. |
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 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. |
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:
|
Release strategy@jisaacks: Continued discussion from #359 When calling the As we will need to add pre-release tags like Tagging suggestion
|
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.) |
You mean:
Thanks for the tip! |
@jisaacks: As
I guess many people may wonder where and why We at least need to publish the information to add the following key to "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:
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. |
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. |
Ok, will rebase the PR branches, autosquash the fixups and generate a PR with the changelog. |
I will create a small PR to improve the readability of the popup css since we now encourage people to change it. |
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:
|
@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 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 No worries, though, and thanks for spear heading all this. But to fix it, I think you need to rename Then I can run Let me know if you have any questions. Thanks! :) |
@deathaxe, Also you don't need to add the |
@deathaxe actually I think I mis spoke. Don't add the tag. That is what my So yeah, not really sure what we should do now since it's likely that everyone will be updated, just without any release notes. |
release message
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. next steps
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.
pre-release namingSeveral 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:
Too indirect, maybe :-) |
I am a bit confused about the packagecontrol.io stats. With GitGutter not in the list of The stats show 0 installs today and only few yesterday. Does this mean everybody in the world has GitGutter included in the list of 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. |
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.
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. |
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:
... as @FichteFoll suggests. |
@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 |
Sounds good to me. |
@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.
The text was updated successfully, but these errors were encountered: