Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM overall. I'm on the fence about staging indexes. Please let me know what you think.
- queue:create-task:highest:scriptworker-prov-v1/mobile-pushapk-dep-v1 | ||
- project:mobile:fenix:releng:signing:cert:dep-signing | ||
- project:mobile:fenix:releng:googleplay:product:fenix:dep | ||
- queue:route:index.project.mobile.fenix.staging-signed-nightly.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure if we want to index these builds. To me, it feels like unnecessary clutter. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, hold on, is this comment also about staging indexes, or is this one about real nightly indexes?
If it's about staging indexes, I think they might provide some value
If it's about real nightly indexes, I think that they provide just as much value for Fenix as they did Focus and Reference Browser :)
artifacts = ["public/{}".format(os.path.basename(apk)) for apk in apks] | ||
|
||
signing_format = 'autograph_apk' | ||
index_release = 'staging-signed-nightly' if is_staging else 'signed-nightly' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question regarding staging indexes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had them initially in there to make sure that creating the indexes was working properly (at which point, once one build happened, the clutter is now in the indexes permanently).
FWIW, they do have a prefix - when looking for real Fenix nightlies, you can go to project.mobile.fenix.signed-nightly
and you'll have moved past the clutter.
On the whole, I like it when staging builds are as similar as possible to real builds (including indexes). For example, if someone refactors decision_task_nightly.py
, they can verify after their refactoring that indexes haven't been broken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Let's keep them for now, then. If nobody complains, then let's keep them forever :)
automation/taskcluster/lib/tasks.py
Outdated
} | ||
} | ||
|
||
def build_signing_task(self, build_task_id, name, description, signing_format, is_staging, apks, scopes, routes): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: build_{}
is ambiguous. It may refer to the build task. Let's change the verb to craft
or another synonym.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can make this change, but we'll need to do a consistency pass across our other repositories (we have so much duplicate code between our three mobile repositories' automation right now, really looking forward to clearing this up)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -1,44 +0,0 @@ | |||
# This Source Code Form is subject to the terms of the Mozilla Public |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to see thigs going away!
Tested in staging, is working up until it attempts to push to Google Play with dummy dep credentials
Can be code-reviewed but is blocked on scriptworker