search

stardot / beebasm

A portable 6502 assembler with BBC Micro style syntax
http://www.retrosoftware.co.uk/wiki/index.php/BeebAsm
GNU General Public License v3.0
83 stars 26 forks source link

Merge `proposed-updates`? #71

Closed chriskillpack closed 1 year ago

chriskillpack commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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. https://github.com/stardot/beebasm/issues/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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

ZornsLemma commented 1 year ago

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 commented 1 year ago

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

ZornsLemma commented 1 year ago

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 commented 1 year ago

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

chriskillpack commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

chriskillpack commented 1 year ago

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

chriskillpack commented 1 year ago

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

ZornsLemma commented 1 year ago

Sounds good to me!

chriskillpack commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

chriskillpack commented 1 year ago

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 commented 1 year ago

I'm happy for it to go ahead.

ZornsLemma commented 1 year ago

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

chriskillpack commented 1 year ago

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.