-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Conversation
6e5cc42
to
ec08d43
Compare
a601c83
to
ffb6558
Compare
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.
Looks pretty good, though I'm 🤔 on running code coverage on release builds.
Regarding secret management, I can add permissions for project/mobile/fenix/debug
, we can land this, then I can do some follow-up work to group PR secrets together afterwards. That way, we can land your changes here. 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.
Looks good! I'll set up permissions
automation/taskcluster/lib/tasks.py
Outdated
def craft_assemble_pr_task(self, variant, run_coverage=False): | ||
coverage_flag = '-Pcoverage ' if run_coverage else '' | ||
assemble_gradle_command = ('{} assemble{}'.format(coverage_flag, variant.for_gradle_command),) | ||
post_gradle_command = ('automation/taskcluster/upload_coverage_report.sh' if run_coverage else '',) |
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.
Understanding the order of operations here is a little tough, but I don't want to block your PR - I can take a look when I update the nimbledroid
secret
I've landed the permissions patch and I'm just getting it applied atm |
The permissions patch is landed and applied, and you can verify that by seeing the |
5564516
to
a3c1e4c
Compare
00da218
to
7b423c9
Compare
d68fb1e
to
acc13d4
Compare
Codecov Report
@@ Coverage Diff @@
## master #4520 +/- ##
========================================
Coverage ? 6.4%
Complexity ? 127
========================================
Files ? 200
Lines ? 8442
Branches ? 1194
========================================
Hits ? 541
Misses ? 7858
Partials ? 43 Continue to review full report at Codecov.
|
I'm testing for a bit. Don't land this until I remove the label.