Skip to content

Fixed function overloading and combining Options with locals. #3

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

Closed
wants to merge 5 commits into from
Closed

Conversation

MichalLytek
Copy link
Contributor

No description provided.

@MichalLytek
Copy link
Contributor Author

MichalLytek commented Sep 12, 2016

I don't know why this pull request has 5 commits when I added now only 2 new commits. Should I merge this repo into my fork first?

@blakeembrey
Copy link
Member

@19majkel94 Because you didn't rebase. You should consider branches as temporary pieces of work and delete them when you're done. I rebased your commit so the history is different to yours, resulting in a conflict.

@felixfbecker
Copy link

Also, never open a PR from your master. Always make a new branch for a PR, which you can then easily rebase on master when master gets updated.

@unional
Copy link

unional commented Sep 12, 2016

@felixfbecker why not from master? Forked master and the source master are just different branches. As long as I work on one PR at a time (which is most of the case for @types or most one-off PR you tries to contribute), I don't see an issue with PR from master.

I'm doing that quite frequently myself, so if there is a good reason not to do it, it will help me to acquire better practice also. 🌷

@blakeembrey
Copy link
Member

blakeembrey commented Sep 12, 2016

@unional I wouldn't promote making a PR from master either. It's unusual to fix the remote tracking of master after a squash of history. Especially for someone new to Git or GitHub.

Also, it's entirely possible that while waiting for your first PR to merge, you'd want to make another. Using master for this just makes things awkward to manage.

@felixfbecker
Copy link

The second you want to start working on another PR or master gets updated it gets messy

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.

4 participants