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

Use GOSUB for primes example & include EXIT #13

Merged
merged 5 commits into from Sep 20, 2018
Merged

Use GOSUB for primes example & include EXIT #13

merged 5 commits into from Sep 20, 2018

Conversation

why-does-ie-still-exist
Copy link
Collaborator

Even after the loop found a factor of x, if would keep going to try and find more. This instead uses EXIT to to end the FOR loop after finding a factor, and also uses GOSUB to make the code more readable(thanks for the suggestion @flagxor)

Also I don't know how to squash commits 'cause I'm a n00b and I'm using the GitHub web interface

Update prime example code to not check extraneous values 😄
Even after the loop found a factor of x, if would keep going to try and find more. This instead uses `EXIT` to to end the `FOR` loop after finding a factor, and also uses GOSUB to make the code more readable(thanks for the suggestion @flagxor)
@why-does-ie-still-exist
Copy link
Collaborator Author

Did it do the conflicts last time?? I'm confused.

@flagxor
Copy link
Collaborator

flagxor commented Sep 20, 2018

So as outlined in this issue I just filed, I instinctively hit github's (relatively) shiny new 'squash and merge' button:
#17

Trouble is exactly the confusion you're hitting. You likely made your change on top of the 3 you'd already done, but in google/wwwbasic, they'd been squished into 1.
Hence the conflict.

Unless somebody screams, I'm thinking for wwwbasic we should go with the merge model, as it's sort of the github way and works well with the web interface. It's only real downside is it keeps everything including all the little mistakes etc., but that's kind of the point of version control.
We'll see what folks say on this issue.

Anyhow, I've resolve the conflict for this one and will vanilla merge it.

I think that will still leave your fork in a mismatched state. Only web ui way I can find to fix that is delete repo and refork, but there might be some cleverness I've missed. You can also fix at the command line by checking out upstream, then force pushing.

Anyhow thanks for the contribution!

@flagxor flagxor merged commit d42b8ae into google:master Sep 20, 2018
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 this pull request may close these issues.

None yet

2 participants