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

Fixup choropleth select test #2109

Merged
merged 1 commit into from
Oct 20, 2017
Merged

Fixup choropleth select test #2109

merged 1 commit into from
Oct 20, 2017

Conversation

etpinard
Copy link
Contributor

Tests are currently failing on master because PR #2099 got merged on a branch behind #2081

In detail, the PR #2099 branch didn't have #2081's addInvisible utility which messed up the trace indices and lead to a failing test.

@monfera please review.

- PR #2099 got merged
  on a branch behind #2081
  which caused the test to fail on master.
@monfera
Copy link
Contributor

monfera commented Oct 20, 2017

Looks great 💃 -thanks for the quick resolution!

Retro: We have a general policy of not rebasing PR branches - I learnt the lesson that this guidance doesn't prohibit us from branching locally and rebasing to see if there's some odd conflict 😄

@etpinard etpinard merged commit f991039 into master Oct 20, 2017
@monfera
Copy link
Contributor

monfera commented Oct 20, 2017

@etpinard What was your secret to fixing it so quickly? I rarely if ever encountered this type of merge sequencing issue where two branches passed tests and merged fine but khmmm.... the person doing the 2nd merge, me, should've rebased in a test branch.

@etpinard
Copy link
Contributor Author

We have a general policy of not rebasing PR branches

Just merge master into your branch if that happens again

image

yes, that adds an extra commit, but it's the least-bad way of doing things that I know of.

@etpinard etpinard deleted the fixup-choropleth-select-test branch October 20, 2017 14:35
This was referenced Oct 20, 2017
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