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

Merge proposed-updates? #71

Closed
chriskillpack opened this issue Aug 16, 2022 · 30 comments · Fixed by #89
Closed

Merge proposed-updates? #71

chriskillpack opened this issue Aug 16, 2022 · 30 comments · Fixed by #89

Comments

@chriskillpack
Copy link
Collaborator

The branch is now 148 commits ahead of master. The oldest commit on the branch is from May 2018 and there has been no activity on master since Feb 2020. It feels like proposed-updates is now the defacto "main" branch.

What testing is required before proposed-updates can be merged?

@mungre
Copy link
Collaborator

mungre commented Aug 16, 2022

The proposed-updates branch does seem to cause confusion so I'd also be inclined to merge it and work on master in the future.

master only has changes to the readme and gitignore so the merge shouldn't break anything in proposed-updates.

proposed-updates passes all the unit tests and I've manually tested it against bogomandel and Prince of Persia. (I really ought to include these in the unit tests somehow.) Anyway, this feels like enough testing to me.

@ZornsLemma
Copy link
Collaborator

ZornsLemma commented Aug 16, 2022

I think I've been largely responsible for the existence of proposed-updates, although it's possible my memory is a bit faulty - it's been a while.

I was using it as a place to accumulate changes so they could be tested together without unilaterally declaring them "official" by merging to master - I think I'd typically announce stuff was on proposed-updates on stardot so people could test/complain while master was still untouched. My hope was that proposed-updates would get merged to master and declared an official release and the version number bumped at some point, I just wanted to give people a chance to look at stuff first..

TL;DR: proposed-updates was there to try to head off any potential drama if someone didn't like one of my changes. :-)

I'm not arguing against doing a merge now, I don't think there's anything controversial on there, just trying to give some historical context. It would be really good if we could get an "official" release out with all the great work that's been done since the last one.

@chriskillpack
Copy link
Collaborator Author

I think the context is useful, thanks.

I propose that a forum announcement is made asking people to do testing and open issues. We can put a deadline in the announcement (say 1 month) after which the merge will happen and a final release cut (and tagged in the repo). The post can include a curated list of changes to entice people to test. I'm happy to make the announcement but I'm not a forum member, is there someone who would usually do this?

Do testers pull the proposed-updates branch themselves or do binaries need to be built ahead of time?

@mungre
Copy link
Collaborator

mungre commented Aug 16, 2022

For Windows binaries I added a github action that automatically builds a release when you tag master with a release number.

You could just tag it with something like "v1.10rc1". It hasn't been used for a year though but hopefully it still works.

Edited to add: also, one of the proposed-updates commits removes beebasm.exe from the repository.

@ZornsLemma
Copy link
Collaborator

ZornsLemma commented Aug 16, 2022

Just from memory, so I may be wrong, but FWIW:

I think for beebasm up to and including 1.08 Rich was actively maintaining the project and he would have made announcements on stardot/retrosoftware.

Things went quiet for a while. I then started working on a project using beebasm quite heavily and had some personal itches to scratch so I made quite a few patches and I probably made announcements on stardot of the kind you're thinking of and (with community approval, or at least no one objecting :-) ) tagged up 1.09 and made it "official". But that was a one-off really.

I would love to see a 1.10 and I think I've made vaguely "shall we do this?" posts on stardot, but I've not had the time or motivation to really push for it. (There are a few lingering bugs in proposed-updates/"1.10" which I always half-hoped to fix, which also put me off. However, these aren't newly introduced in 1.10 so there's no real reason they should hold up a release.) If you're willing to join the forum I'd be very happy for you to make this announcement; I never considered myself any kind of (un)official release manager or anything like that, I was just keen for 1.09 to exist officially so I could say "use beebasm 1.09" in my project's README, rather than saying "build beebasm from branch X in my repo, otherwise it won't be able to build this project successfully".

@ZornsLemma
Copy link
Collaborator

PS There are a few possible enhancements which are coded up and potentially ready to merge, but which haven't been merged to proposed-updates yet, e.g. #39. At this point I don't recall exactly why I didn't merge these, perhaps I just never got round to it - that one at least doesn't look particularly controversial. It might be worth making a pass over the open issues to look for easy stuff like this before making an announcement about a candidate 1.10; swings and roundabouts really, we might introduce some breakage in the process. (I'm happy to go over the open issues and post some candidates here for discussion if you like.)

@ZornsLemma
Copy link
Collaborator

I've had a quick look over the other issues and the following might be of interest for 1.10 but aren't yet on proposed updates:

@chriskillpack
Copy link
Collaborator Author

Thanks for going through the list, IMO the ones you picked out aren't release blockers for me. I think it would be good to get v1.10 out so proposed-updates can be retired and all these changes can be made on master. Thoughts?

@ZornsLemma
Copy link
Collaborator

I definitely agree for #63 and #36. For #42 I can't help thinking that it's an isolated change (if the new "-dd" option produces junk, it's not exactly a regression) and it might be nice to get this feature in given the code exists and has been sitting around for ages. I don't have tremendously strong feelings about this though.

@mungre
Copy link
Collaborator

mungre commented Aug 18, 2022

#36 has always bothered me so I've done a fix in #73, which also covers #48. If there are no objections I'll merge it in.

#59 has been sitting around for a year and is ready to merge in. Again, if there are no objections I'll merge it but I don't really care about this one.

Otherwise, I'm happy to go ahead with a release.

@chriskillpack
Copy link
Collaborator Author

Can someone give me write access to this repo so I can create the tag and create a release?

@ZornsLemma
Copy link
Collaborator

I think I can do it, although equally I don't think I ever actually have so it might take some figuring out..

Since this isn't my repo, can you please post over in the relevant thread on stardot at https://stardot.org.uk/forums/viewtopic.php?f=55&t=12203&start=60 for form's sake? I'll keep an eye on that and add you when I see your post.

Assuming I can do either, do you want write access to just this repo or do you want to be a member of the "stardot" organisation?

@chriskillpack
Copy link
Collaborator Author

Requested, I picked org access because that's what everyone else was doing.

@ZornsLemma
Copy link
Collaborator

The best reason for doing anything. :-) I've just invited you, I infer from some text during the process of inviting you that we'll need to grant you write access to the repo separately after you accept the invitation. But I'm no expert on github permissions...

@ZornsLemma
Copy link
Collaborator

Right, I see you've accepted the invitation and I think I've just granted you write access to this repository.

@chriskillpack
Copy link
Collaborator Author

Ty. I was able to tag a release candidate and the GH Action created a release. I don't have access to a Windows computer, can someone give it a quick check? If all is well then I will send out the announcement post on *. later

@mungre
Copy link
Collaborator

mungre commented Aug 21, 2022

It seems to work fine but proposed-updates hasn't been merged into master so there are a couple of changes missing. Can you do the merge and create a release candidate 2?

@chriskillpack
Copy link
Collaborator Author

I don't think there are any missing commits from master (upstream is what I've called the main repo in my fork)

$ git fetch upstream
$ git merge-base upstream/master upstream/proposed-updates
77609cc67880fd7cc61ef6875a00dcb20ee8a9bd
$ git rev-parse upstream/master
77609cc67880fd7cc61ef6875a00dcb20ee8a9bd
$ git log 77609cc67880fd7cc61ef6875a00dcb20ee8a9bd..upstream/master
$

What this says is that the branch off point for proposed-updates is commit 77609cc67880fd7cc61ef6875a00dcb20ee8a9bd which is the also upstream/master. So I don't think there are any commits from master missing from proposed-updates, and hence nothing missing in v1.10rc1. LMK if I have this wrong.

@mungre
Copy link
Collaborator

mungre commented Aug 22, 2022

You're absolutely right. I'm sorry about that. I hadn't noticed that Steve had merged those changes from master back into proposed-updates. I did double-check and thought the README change wasn't incorporated, but I misread it. Carry on!

@mungre
Copy link
Collaborator

mungre commented Aug 22, 2022

(And I don't mean I'm sorry you're right! I'm sorry for wasting your time.)

@chriskillpack
Copy link
Collaborator Author

Haha, no I understood what you were apologizing for. Forum post is up

@chriskillpack
Copy link
Collaborator Author

With the COPYBLOCK fix and the -cycles change I think we should cut another release candidate. Any disagreement?

@ZornsLemma
Copy link
Collaborator

Sounds good to me!

@chriskillpack
Copy link
Collaborator Author

Between rc2 and HEAD of proposed-updates the only runtime change is PR #84. What do people think about either issuing another release candidate, or merging proposed-updates and releasing v1.10? In my forum post I gave people until October but that looks to be overly generous.

@mungre
Copy link
Collaborator

mungre commented Sep 10, 2022

If we're going to compress the timescale it wouldn't hurt to give, say, a week's notice on stardot. And then we might as well do a final RC.

@chriskillpack
Copy link
Collaborator Author

Okay well I'm happy to wait and stick with our original release schedule.

@chriskillpack
Copy link
Collaborator Author

Well it is October, I'm happy to release what we have on this branch as v1.10. I think the one change not in v1.10rc2 looks low-risk and not requiring a separate RC. Thoughts?

@mungre
Copy link
Collaborator

mungre commented Oct 4, 2022

I'm happy for it to go ahead.

@ZornsLemma
Copy link
Collaborator

I think it's reasonable to go ahead, I haven't seen any reports of problems on stardot or anything like that.

@chriskillpack chriskillpack linked a pull request Oct 4, 2022 that will close this issue
@chriskillpack
Copy link
Collaborator Author

chriskillpack commented Oct 4, 2022

I decided to make a PR instead of doing the git merge manually, so it could be given one last look over. I pushed a commit to bump version to v1.10. LMK.

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

Successfully merging a pull request may close this issue.

3 participants