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

-writes should be -cycle #74

Closed
jgharston opened this issue Aug 22, 2022 · 10 comments
Closed

-writes should be -cycle #74

jgharston opened this issue Aug 22, 2022 · 10 comments

Comments

@jgharston
Copy link

I was wondering what the "sets the write value" meant and dug through the documentation.

Ah, it sets the disk cycle. That's what it's called, you should specify it by what it's actually called, as with other tools, and to match the documentated nomenclature, eg https://mdfs.net/Docs/Comp/Disk/Format/DFS

@chriskillpack
Copy link
Collaborator

@ZornsLemma WDYT?

@ZornsLemma
Copy link
Collaborator

It looks like tricky's happy to change it (https://stardot.org.uk/forums/viewtopic.php?p=368483#p368483) and "cycle" seems a decent name to me. Do you want me to tweak this on proposed-updates?

@ZornsLemma
Copy link
Collaborator

I've done this on a branch for now: https://github.com/stardot/beebasm/tree/issue-74

@chriskillpack
Copy link
Collaborator

Want to make this a PR and then link to the issue? Once the PR is merged into proposed-updates it will automatically close this issue.

BTW Why does your commit message include a copy of the diff output?

@ZornsLemma
Copy link
Collaborator

I don't know why the diff output is there; I did "git commit -av" but I nearly always do that and the man page says that diff doesn't get included in the commit message. My best guess is that I accidentally deleted a blank line or something. I'll create a new issue-74b branch with that fixed and do a pull request for that.

@ZornsLemma
Copy link
Collaborator

ZornsLemma commented Aug 26, 2022

Right, I think I got there in the end: #77

I don't know if we should delete the issue-74 and issue-74b branches with the bad commit message on - what do you think? issue-74c is the one mentioned in the pull request.

@mungre
Copy link
Collaborator

mungre commented Aug 26, 2022

The change looks good. Thanks for doing this.

I'd definitely delete the duff branches. I've also deleted the source branches for the last couple of PRs I merged because the list of branches was getting rather long, but that's not a great rationale.

chriskillpack added a commit that referenced this issue Aug 27, 2022
Merge fix for issue #74 (change -writes option to -cycle)
@chriskillpack
Copy link
Collaborator

I merged it in, thanks for doing it.

I asked about diff output in commit message because that has happened to me before, but only when I was making changes in a repo where the authors primarily worked in Windows (e.g. CR/LF) and I was making changes on a Mac. I'm going to blame a combination of me not checking, my editor and finally git.

@chriskillpack
Copy link
Collaborator

Closing this issue. We will include this in Release Candidate 2.

@ZornsLemma
Copy link
Collaborator

Thanks. Just to note I've just deleted the duff issue-74 and issue-74b branches (with the bad commit message; the code change was fine). I've left issue-74c (the one that actually got merged), as mungre says maybe we should clean up more branches later but I didn't want to get too carried away right now.

I am on Linux and it's vaguely possible line-ending stuff played a role in the diff getting include in the commit message. I'll try to see if I can spot a pattern if it happens again...

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

4 participants