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

Switch releases to taskgraph #3168

Merged
merged 1 commit into from
Jun 23, 2020
Merged

Switch releases to taskgraph #3168

merged 1 commit into from
Jun 23, 2020

Conversation

MihaiTabara
Copy link
Contributor

@MihaiTabara MihaiTabara commented May 27, 2020

Last PR that references #1945 and adds taskgraph support for releases.

@MihaiTabara MihaiTabara added the WIP Work in progress label May 27, 2020
@MihaiTabara MihaiTabara self-assigned this May 27, 2020
@firefoxci-taskcluster
Copy link

Uh oh! Looks like an error! Details

bad indentation of a mapping entry at line 186, column 29:
else:
^

2 similar comments
@firefoxci-taskcluster
Copy link

Uh oh! Looks like an error! Details

bad indentation of a mapping entry at line 186, column 29:
else:
^

@firefoxci-taskcluster
Copy link

Uh oh! Looks like an error! Details

bad indentation of a mapping entry at line 186, column 29:
else:
^

.taskcluster.yml Show resolved Hide resolved
.taskcluster.yml Show resolved Hide resolved
.taskcluster.yml Show resolved Hide resolved
.taskcluster.yml Show resolved Hide resolved
.taskcluster.yml Show resolved Hide resolved
.taskcluster.yml Show resolved Hide resolved
.taskcluster.yml Show resolved Hide resolved
.taskcluster.yml Show resolved Hide resolved
@firefoxci-taskcluster
Copy link

Uh oh! Looks like an error! Details

bad indentation of a mapping entry at line 186, column 29:
else:
^

1 similar comment
@firefoxci-taskcluster
Copy link

Uh oh! Looks like an error! Details

bad indentation of a mapping entry at line 186, column 29:
else:
^

taskcluster/app_services_taskgraph/__init__.py Outdated Show resolved Hide resolved
taskcluster/ci/config.yml Show resolved Hide resolved
@@ -0,0 +1,9 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: I'll remove this before merging.

@MihaiTabara
Copy link
Contributor Author

I think this is ready for a first round of review r? @JohanLorenzo

Copy link
Contributor

@JohanLorenzo JohanLorenzo left a comment

Choose a reason for hiding this comment

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

Looks 👍 to me, 🚢 it!

.taskcluster.yml Show resolved Hide resolved
automation/upload_android_symbols.sh Outdated Show resolved Hide resolved
taskcluster/app_services_taskgraph/__init__.py Outdated Show resolved Hide resolved
@MihaiTabara MihaiTabara requested a review from eoger June 22, 2020 17:16
@MihaiTabara MihaiTabara removed the WIP Work in progress label Jun 22, 2020
@MihaiTabara MihaiTabara changed the title Switch releases to taskgraph [ci skip] Switch releases to taskgraph Jun 22, 2020
@MihaiTabara MihaiTabara reopened this Jun 22, 2020
@MihaiTabara MihaiTabara changed the title [ci skip] Switch releases to taskgraph [ci full] Switch releases to taskgraph Jun 22, 2020
@MihaiTabara MihaiTabara changed the title [ci full] Switch releases to taskgraph Switch releases to taskgraph Jun 22, 2020
@MihaiTabara
Copy link
Contributor Author

@eoger This is ready for review & merge whenever we can coordinate.

Staging releases, ci skip, ci full and normal PRs works as expected.

eoger
eoger previously approved these changes Jun 22, 2020
Copy link
Contributor

@eoger eoger left a comment

Choose a reason for hiding this comment

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

Looks ok to me, let's do the migration tomorrow. Most important thing for me is that we can revert the changes as soon as we detect a problem since not being able to release quickly would leave us in a bad position.

  • I assume we'll keep the "legacy" TC scopes till we're 100% sure the transition was successful?
  • Are you able to rebase and squash these changes? I will be way easier to revert 1 commit if things go wrong.

.taskcluster.yml Outdated Show resolved Hide resolved
@@ -5,29 +5,32 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume symbols uploading is still OK?

Copy link
Contributor Author

@MihaiTabara MihaiTabara Jun 23, 2020

Choose a reason for hiding this comment

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

Yes. I just moved the way we consume credentials. Tested locally (up until getting 401 for auth credentials) and in staging releases. Unfortunately, I can't test production push until we actually merge but all the local graphs suggest it's working smooth.

@MihaiTabara
Copy link
Contributor Author

MihaiTabara commented Jun 23, 2020

Looks ok to me, let's do the migration tomorrow. Most important thing for me is that we can revert the changes as soon as we detect a problem since not being able to release quickly would leave us in a bad position.

* I assume we'll keep the "legacy" TC scopes till we're 100% sure the transition was successful?

* Are you able to rebase and squash these changes? I will be way easier to revert 1 commit if things go wrong.
  1. Agreed to revert things. Worse comes to worst, we revert the PR and go back to what we had.
  2. legacy TC scopes, CoT product and everything else are staying for at least 2-3 releases to confirm things are working fine with the new world. That's why I didn't remove the old logic either (from automation/taskcluster). For now, I'm keeping it there until we're safe and then cleanup in a few weeks or so.
  3. Not sure what you mean re: squashing - I already did that - it's just one commit in this PR. Maybe you reviewed before I pushed my lattest changes 🤔

@mergify mergify bot dismissed eoger’s stale review June 23, 2020 08:07

The pull request has been modified, dismissing previous reviews.

@MihaiTabara MihaiTabara requested a review from eoger June 23, 2020 08:09
Copy link
Contributor

@eoger eoger left a comment

Choose a reason for hiding this comment

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

It's merge time!

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.

3 participants