Skip to content
This repository was archived by the owner on Jan 29, 2025. It is now read-only.

No migrations left behind, in CI#1591

Merged
bors[bot] merged 3 commits into
mozilla:masterfrom
peterbe:1590-no-migrations-left-behind-in-ci
Oct 19, 2018
Merged

No migrations left behind, in CI#1591
bors[bot] merged 3 commits into
mozilla:masterfrom
peterbe:1590-no-migrations-left-behind-in-ci

Conversation

@peterbe
Copy link
Copy Markdown
Contributor

@peterbe peterbe commented Oct 18, 2018

Fixes #1590

peterbe
peterbe commented Oct 18, 2018
View reviewed changes
Comment thread .circleci/config.yml

environment:
COMPOSE_FILE: ci/docker-compose.yml
COMPOSE_FILE: ci/docker-compose.yml
Copy link
Copy Markdown
Contributor Author

@peterbe peterbe Oct 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My vscode fixes yaml files automatically and although this is unrelated I don't think it hurts to correct.

Copy link
Copy Markdown
Contributor

@mythmon mythmon Oct 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future it would be good to only include these kind of fixes to files that are already being changed in the PR. This change is more correct so lets keep it, but if we're going to have automatic fixes for these kind of files, we should add them as rules to Therapist instead of ad-hoc changes.

@peterbe peterbe added the blocked Depends on something else label Oct 18, 2018
@peterbe
Copy link
Copy Markdown
Contributor Author

peterbe commented Oct 18, 2018

I'm expecting this to fail in CircleCI. Running it now to confirm that.

Once #1588 is resolved we can update this one and it should pass.

mythmon
mythmon approved these changes Oct 18, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@mythmon mythmon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

As a note, once #1588 lands, we can bors r+ this PR without rebasing it, since Bors will merge the two changes together.

Comment thread .circleci/config.yml

environment:
COMPOSE_FILE: ci/docker-compose.yml
COMPOSE_FILE: ci/docker-compose.yml
Copy link
Copy Markdown
Contributor

@mythmon mythmon Oct 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future it would be good to only include these kind of fixes to files that are already being changed in the PR. This change is more correct so lets keep it, but if we're going to have automatic fixes for these kind of files, we should add them as rules to Therapist instead of ad-hoc changes.

mythmon
mythmon approved these changes Oct 19, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@mythmon mythmon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The blocking issues are fixed now.

bors r+

@mythmon mythmon removed the blocked Depends on something else label Oct 19, 2018
bors Bot added a commit that referenced this pull request Oct 19, 2018
@bors @peterbe
Merge #1591
766d24b 
1591: No migrations left behind, in CI r=mythmon a=peterbe

Fixes #1590

Co-authored-by: Peter Bengtsson <mail@peterbe.com>
@bors
Copy link
Copy Markdown
Contributor

bors Bot commented Oct 19, 2018

Build succeeded

@bors bors Bot merged commit 328dd1f into mozilla:master Oct 19, 2018
@peterbe peterbe deleted the 1590-no-migrations-left-behind-in-ci branch October 19, 2018 20:17
This was referenced Mar 19, 2019
Merged
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@mythmon mythmon mythmon approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@peterbe @mythmon