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

Build/install netime, dmpcpy, and pfthmg daemons; add link sys; ts p (to sys; ts peek) #9

Merged
merged 2 commits into from
Nov 18, 2016

Conversation

eswenson1
Copy link
Member

  • Added channa and dragon directories, and added sources for netime, dmpcpy, fsdefs, modems, netwrk, and pft.
  • Build/install netime, dmpcpy, and pfthmg daemons
  • Add link to sys; ts peek as sys; ts p

- Build/install netime, dmpcpy, and pfthmg daemons
- Add link to sys; ts peek as sys; ts p
@larsbrinkhoff
Copy link
Member

The first of these two commits is incomplete. It tries to add two empty directories to the source tape, but itstar doesn't like that. I suppose a DUMP tape can't contain empty directories.

It's nice if every single commit is self-contained and don't fail the build. Having said that, I don't think we need to be that stringent. I'l merge this as is.

@larsbrinkhoff larsbrinkhoff merged commit 336bbaa into master Nov 18, 2016
@eswenson1
Copy link
Member Author

How can I make this be true, when, for example I miss a file, in a local commit, or have a bug and fix locally before pushing? Or if I push twice to GitHub? I only submitted one PR (to encompass the two commits/pushes). Are you suggesting that before pushing that I consolidate all local commits? (I've actually never done that with Git before, but believe I've seen documented how to do that). Please advise.

-- Eric

On Nov 18, 2016, at 10:27, Lars Brinkhoff notifications@github.com wrote:

Merged #9.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@larsbrinkhoff
Copy link
Member

It's possible to completely rewrite pull requests. You can do that with gits infamous git rebase -i master.

I'm a bit fanatic when it comes to submit squeak-clean pull requests. If I notice an error, I can add or remove or reorder commits, or edit them, or whatever it takes.

Another option besides rebasing, is to do git reset master. This will remove all commits, and reset the branch back to the state of the master branch but leave all changes in place. This way, you can start over and make new commits.

I also use git add -p frequently to just add parts of a file to the staging area.

A pull request may well consist of several commits. I think it's good practice to make each commit self-contained and focused on one simple task. So e.g. your TS PEEK link could have been a separate commit.

@eswenson1
Copy link
Member Author

Hi Lars,

I’m still a bit confused. I submitted a single pull request, and expected it to encompass the two commits I had pushed to my branch. What should I have done differently in the situation where I first tested my tree locally and found it working fine, then did a “commit” that missed a few files, the fixed this by issuing another commit? In fact, I did do two “pushes”, since I realized after the first push that I forgot some files. I realize that this caused Travis to perform two builds, the first, which should have failed, and the second, which should have succeded. It is usually difficult to know that a commit is incomplete, since the build works fine locally, but won’t when pulled via CI. True, a “git status” would have revealed the missing files before I did a “git push”, but I can’t believe that, in general, this doesn’t happen in the real world. People often have to “fix” a commit/push when they realize they forgot something or had a bug that only shows up in CI.

As far as I know, I should have had a “squeaky-clean” pull request. I submitted only one pull request, which should have encompassed both commits. What did I do wrong? I authored the pull request, and waited until Travis said everything was ok and then I submitted the request. In fact, I didn’t see any failed build there — it showed everything good. What should I have done differently?

Regarding making the TS PEEK link a separate commit — understood. I’ll be more careful.

But please do commend on the issue of a “single pull request” that encompasses two commits, because I may have a big misunderstanding regarding Git that, although I’ve used Git for several years now, I’ve somehow missed! :-)

— Eric

On Nov 18, 2016, at 10:42, Lars Brinkhoff notifications@github.com wrote:

It's possible to completely rewrite pull requests. You can do that with gits infamous git rebase -i master.

I'm a bit fanatic when it comes to submit squeak-clean pull requests. If I notice an error, I can add or remove or reorder commits, or edit them, or whatever it takes.

Another option besides rebasing, is to do git reset master. This will remove all commits, and reset the branch back to the state of the master branch but leave all changes in place. This way, you can start over and make new commits.

I also use git add -p frequently to just add parts of a file to the staging area.

A pull request may well consist of several commits. I think it's good practice to make each commit self-contained and focused on one simple task. So e.g. your TS PEEK link could have been a separate commit.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub #9 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AA0-YxQ0kle2sSbcrsOre-1tDSOE3Eq0ks5q_fF_gaJpZM4K2p23.

@larsbrinkhoff
Copy link
Member

Sure, I'm happy to go on at length about this, if you're willing to listen! I just hope I can explain in a way that makes sense.

You did one commit, pushed it, and made pull request. Then you realized something was amiss, and you wanted to fix it. You made another commit and pushed that. Travis is happy.

What I would have done at this point, is to go back and fix the first failing commit. I would edit the commit so the build no longer failed. When everything is done, I would do git push -f to forcibly update the branch. GitHub will notice this, and update the pull request accordingly.

I have learned these practices from the open-source world. Linux, GCC, binutils, etc, all do one form or another of this. Twenty years ago, there was no git, and changes where submitted as a series of patches to a mailing list. The changes were scrutinized by the maintainers, and if anything in a patch was bad, the whole series was turned down, and you were asked to fix it and resubmit a new series. This is much like how GitHub works today. A pull request is like a patch series, and a commit is like a patch.

Why go to all this trouble? Because often, a bug surfaces, and you will have to back to a commit and try to figure out what is does, and the intent behind it. If a commit is simple, this makes the job much easier. If a commit is broken, and relies on another commit to make it work, it's harder to understand. You also have to study the other commit.

@larsbrinkhoff
Copy link
Member

Original source for DRAGON:
SYSENG; DRAGON 63

@larsbrinkhoff
Copy link
Member

Looks like SYSENG; DRAGON 63 is very different from your SYSENG; DRAGON 206.

The older one is much larger, and it doesn't mention TARAKA.

@larsbrinkhoff
Copy link
Member

DRAGON 63 quite plainly says it's Puff. So it's not that DRAGON.

I compared TARAKA 14 against CHANNA; ATSIGN TARAKA, which is the file linked to by SYS; ATSIGN DRAGON. I think it's a match! ATSIGN TARAKA disassembly shows the symbol VERSIO has the SIXBIT value 14.

@eswenson1, how did you come up with the version number for your reconstructed SYSENG; DRAGON 206?

@eswenson1
Copy link
Member Author

I have no recollection of how this became 206. Glad you found the original!

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

Successfully merging this pull request may close these issues.

2 participants