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

2.1.0 Release #114

Merged
merged 128 commits into from Oct 12, 2017
Merged

2.1.0 Release #114

merged 128 commits into from Oct 12, 2017

Conversation

@burnsauce
Copy link
Contributor

@burnsauce burnsauce commented Oct 11, 2017

What does this PR do?

Updates master branch to 2.1.0

Provide links to any related discussion on lines.

https://llllllll.co/t/teletype-2-1-firmware-beta-rc2-every-et-al/9192

How should this be manually tested?

Use the new features.

Any background context you want to provide?

All design choices and features have been approved by @tehn

If the related Github issues aren't referenced in your commits, please link to them here.

EVERY implementation fixes #111
I think the rest have been referenced by number.

I have,

  • [ x ] updated CHANGELOG.md
  • [ x ] updated the documentation
burnsauce added 30 commits Sep 3, 2017
Added an initialization flag to the scene state to denote when a scene
is first loading.  SCENE operator aborts if the flag is present.
Buffered input
Clear-on-new-entry
Apply-on-Enter
Abort-on-navigation
Incrementers wrap around 16-bit limits
Number entry blocks if it would exceed limits
Now tracking script number in the execution state and script last run
times in the script state.

One line tap tempo (hit F1 twice):
1: M LAST
M: TR.P 1
AVG -32767 -32768 == -32767
AVG 32767 32766 == 32767
Maximum depth set by WHILE_DEPTH (default 10000)
SCRIPT was breaking W.  Now it doesn't.
If conditions and for loop iterators do not transcend their execution
depth.

Live mode now behaves like this:

> L 1 4: A I
> A => 4
> I => 0

I is 0 because it exists in a different context than the first command.
As a result, the code is cleaner and easier to read.

Short story: only SCRIPT should call es_push and es_pop
Conflicts:
	docs/ops/controlflow.toml
	src/state.c
	src/state.h
	src/teletype.c
@burnsauce
Copy link
Contributor Author

@burnsauce burnsauce commented Oct 12, 2017

Yes, I'm done making changes to this branch. If bugs get discovered past this point, that's what the third digit in the release number is for! ;)

Merge away!

@tehn tehn merged commit ad358cd into monome:master Oct 12, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@samdoshi
Copy link
Collaborator

@samdoshi samdoshi commented Oct 13, 2017

@tehn did this get "squash merged"?

@tehn
Copy link
Member

@tehn tehn commented Oct 13, 2017

@samdoshi
Copy link
Collaborator

@samdoshi samdoshi commented Oct 13, 2017

Did you mean to "squash" it? (i.e. turn the >100 commits into a single one)

Be careful, GitHub changed the merge box on PRs to give 3 options.

@tehn
Copy link
Member

@tehn tehn commented Oct 13, 2017

@samdoshi
Copy link
Collaborator

@samdoshi samdoshi commented Oct 13, 2017

Usually yes. Sometimes you want to use the other options.

Also, it's messed up the contributors graph a bit, so @burnsauce doesn't get quite as much recognition on it as he deserves. (Though we all know how much work he has put into it!)

@burnsauce
Copy link
Contributor Author

@burnsauce burnsauce commented Oct 15, 2017

I must say that I was looking forward to seeing my 200+ commits appear on the graph.

It's hard for me to put forward technical arguments as to why it shouldn't have been squashed because they would be disingenuous in light of that.

Just posting this here to voice my opinion before too many more commits stack up and it might be harder to redo the PR.

@scanner-darkly
Copy link
Member

@scanner-darkly scanner-darkly commented Oct 15, 2017

i think recognizing everybody's contributions is super important. but feels like commit history should be more driven by what makes it more useful/efficient for us. i had commits that were some silly typo or such, i wouldn't want them creating noise on the master branch. but yeah i'm not sure how to balance it with trackability... is there a way to cherry pick what to squash? interesting question this...

@samdoshi
Copy link
Collaborator

@samdoshi samdoshi commented Oct 15, 2017

So firstly, if this had happened to me, I would feel pretty hacked off. By all metrics @burnsauce should be the 2nd highest contributor to the repo behind me.

That commit history will also be really helpful with blame and bisect. Really helpful.

The only way to solve this now is to do some history rewriting and then force pushing.

I'm pretty sure that I can do that, by branching of burnsauce/teletype and cherry picking the newer commits on top of it, but, the hardest bit will be for @scanner-darkly to do. He's already rebased his Teletype OPs branch on top of this PR.

So basically, we're stuck between a rock and a hard place. Whatever we do, I think we should decide quickly.

@samdoshi
Copy link
Collaborator

@samdoshi samdoshi commented Oct 15, 2017

Right one more option, the following branch:

https://github.com/samdoshi/teletype/tree/burnsauce-master

Has all of @burnsauce commits merged back in, because they're merged in there is no need to force push. But it does make the commit history look really really wonky.

Here is the diff:

master...samdoshi:burnsauce-master

You can see all the commits, but also see that it doesn't change anything.

Here is the blame:

https://github.com/samdoshi/teletype/blame/burnsauce-master/src/turtle.c

You can see that all the original commits are showing up.

@burnsauce
Copy link
Contributor Author

@burnsauce burnsauce commented Oct 15, 2017

@samdoshi Thank you for following up on this!

I really appreciate the work and attention that you put into this, especially considering your limited time.

Also I apologise for claiming it's 200+ commits, as it's only 100+. I think I might be developing dyslexia of some sort. Numeric transposition has been happening to me with alarming regularity.

@tehn
Copy link
Member

@tehn tehn commented Oct 15, 2017

@samdoshi
Copy link
Collaborator

@samdoshi samdoshi commented Oct 15, 2017

@tehn yeah GitHub changed the UI, the default option seems to change, so you really need to focus on it. It's good that they offer the options now, but it is confusing.

Shall I open up a PR to merge the commits back in then? I think it works, but it's a bit at the edge of my git knowledge. @cmcavoy / @jlmitch5 either of you have any insight?

@tehn
Copy link
Member

@tehn tehn commented Oct 15, 2017

@burnsauce
Copy link
Contributor Author

@burnsauce burnsauce commented Oct 15, 2017

Thank you both for taking the time! Sorry to @scanner-darkly for any additional work this will bestow upon your grid fork :(

@jlmitch5
Copy link
Contributor

@jlmitch5 jlmitch5 commented Oct 15, 2017

Agreed the expanded commit history is very useful (especially for someone, like myself, using this repo as a way to learn more about C/C++ embedded dev).

I don't really understand what you did with that PR @samdoshi, but it looks to be working great! I would say this way is especially good because it might be weird to rewrite history with force pushes since the master branch has releases off of it. I don't know what would happen to them.

That brings up a point...I guess this would be merged into master, so it wouldn't be present in the 2.1.0 release tag, but would be for all subsequent releases off of master. I assume that's fine just wanted to bring it up.

@burnsauce
Copy link
Contributor Author

@burnsauce burnsauce commented Oct 15, 2017

Somewhat painful news: the prune job I did removing the .zip files and .hex files that I once added to the repo did not take.

I only found this out by checking burnsauce-master@samdoshi/teletype with BFG which means that I did push those things onto the branch.

Seems that history will need to be rewritten at least for 2.1.0. Sorry that my git-newbieness has dirtied the repo.

For reference, I used https://rtyley.github.io/bfg-repo-cleaner/ with

java -jar bfg.jar -D '*.zip'
git reflog expire --expire=now --all && git gc --prune=now --aggressive
java -jar bfg.jar -D '*.hex'
git reflog expire --expire=now --all && git gc --prune=now --aggressive

and it found stuff that I thought I had already deleted with git filter-branch.

:(

@samdoshi
Copy link
Collaborator

@samdoshi samdoshi commented Oct 15, 2017

@burnsauce get a cleaned up branch sorted for me, then I can take it from there.

Because @tehn squash merged it, none of the zip files are in the current master. If you can get a cleaned up branch, I can redo the merge and open up a PR. If it's too difficult to clean up your branch, then I suggest we keep master as is, and chalk this one up to experience.

@burnsauce
Copy link
Contributor Author

@burnsauce burnsauce commented Oct 15, 2017

My master has been cleaned of these references.

Deleted files
-------------

        Filename                 Git id                                                       
        --------------------------------------------------------------------------------------
        teletype-2.1-beta1.zip | 718fc9e8 (414.8 KB), 74e18c23 (414.8 KB)                     
        teletype-2.1-beta2.zip | 2d940c8c (415.2 KB)                                          
        teletype-2.1-beta3.zip | 6c674ff8 (411.8 KB)                                          
        teletype-2.1-beta4.zip | c42995be (412.3 KB)                                          
        teletype-2.1-beta5.zip | 1fe08931 (412.5 KB), 274b0a97 (412.2 KB), fb8421c7 (412.3 KB)

Deleted files
-------------

        Filename       Git id                                                       
        ----------------------------------------------------------------------------
        teletype.hex | c341ec91 (345.6 KB), 1f9f47f0 (331.5 KB), 3f9f4909 (346.2 KB)

Edit: I have gained a lot of experience over the past month, but I still have much to learn. Thank you for your patience.

@samdoshi
Copy link
Collaborator

@samdoshi samdoshi commented Oct 15, 2017

Can you check for PDF files too?

@burnsauce
Copy link
Contributor Author

@burnsauce burnsauce commented Oct 15, 2017

No PDF files found! :)

@scanner-darkly
Copy link
Member

@scanner-darkly scanner-darkly commented Oct 15, 2017

for my grid2 branch i could probably just rebase off whatever the master ends up with? this shouldn't be a problem if i understand correctly, it should be able to replay my changes with no conflicts.

and i think we should have a discussion on commit etiquette - blame and tracking is very useful, but maybe that means putting more care into what goes into a commit - when i start development i might have a bunch of commits that is work in progress/just me forgetting to do things/silly typos, i don't think those would be useful for blame and would just create more noise for others. thoughts?

@burnsauce
Copy link
Contributor Author

@burnsauce burnsauce commented Oct 15, 2017

If there were an easy way to selectively roll up a sequential series of commits into one after the fact, that would allow one to hide typo and other premature commit fixes while retaining the richness of the commit metadata.

Is that something git can do?

@samdoshi
Copy link
Collaborator

@samdoshi samdoshi commented Oct 15, 2017

Yes, you need to Google "git interactive rebase". Read through a couple of the links to get an idea of how it works.

One thing to remember with git, is that even when rewriting history, old commits are never lost (not until the garbage collector runs, and that is pretty infrequent), but they can be hard to easily find again. So whenever I'm about to do something tricky with history in git, I always like to make a branch (e.g. git branch pre-rebase), so that I can always get back to where I started when it all goes horribly wrong!

(I've had a busy day, so I'm not going to be able to doing any more on this today. So if you want to use the time to rewrite history a bit, go for it.)

@jlmitch5
Copy link
Contributor

@jlmitch5 jlmitch5 commented Oct 15, 2017

Interactive rebase/squashing is a good option.

A couple other notes (sorry if this is all super obvious):

  • It is good practice to get pull --rebase off the branch you're basing off of. This keeps your changes on top of the current state, even if there are commits in the branch getting pulled that are more recent than some of your commits. This keeps your changes as a section that can cleanly go back in the tree.
  • soft reset can be really useful. get reset head~n will unstage the n top-most commits. You can then commit pieces of the unstaged changes in a more semantic way (use git add -p to run through the unstaged changes interactively). I typically find this easier to work with that interactive rebasing, but I think it depends on the type of changes...this would probably be pretty difficult with a single PR of a very large amount of commits. You will have to force push to your remote after doing this. Typically it's okay to rewrite history before it goes in the mainline (i.e. master in this case). Once it's in there, it becomes a lot more of a tricky proposition (not as big of a deal with the teletype repo as it's not a project with 100s of collaborators.)

I always like to make a branch (e.g. git branch pre-rebase), so that I can always get back to where I started when it all goes horribly wrong!

This is very good advice, and really, any time you are about to do git push --force to rewrite a remote it's a good idea to have the changes somewhere so you don't accidentally lose them.

@burnsauce
Copy link
Contributor Author

@burnsauce burnsauce commented Oct 15, 2017

I gave rewriting the history for my changes a shot, but the squashes are less automatic than I expected, leading me to have to manually fix merges. As this could lead to bugs, I'm not going to engage in this activity at this point in time.

Knowing how to squash will lead me to cleaner commit logs in the future. I have no interest in creating noise, and would consider a feature/bugfix PLUS its documentation 1 commit.

@samdoshi, you can consider my master branch as ready to go whenever you want to commit surgery. Thank you all for the guidance.

samdoshi added a commit that referenced this pull request Oct 18, 2017
Re-add commits lost due to squashing of PR #114
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

6 participants