[HACK WEEK] Add Dependabot & Dependency Tree Diff Configuration by ParaskP7 · Pull Request #16092 · wordpress-mobile/WordPress-Android

ParaskP7 · 2022-03-11T14:33:58Z

This PR adds Dependabot and Dependency Tree Diff tooling and related configuration to this repo. I chose to add both tools in one go because Dependabot will very much benefit from Dependency Tree Diff and ultimately help reviewers do their review more effectively.

Also as part of this work I created the following:

New bot: dependencies update label (see Bot Labels query). Dependabot is going to be using this label (see dependabot.yml configuration)
New Android Developers team for the WordPress Mobile organization (see Android Teams query). Dependabot is going to be using this team as reviewers (see dependabot.yml configuration)

As I am mainly copy-pasted the already existing configuration from WCAndroid. I tried to be careful with the setup but I might be missing something obvious. As such, I am shamelessly mentioning @wzieba (for on Dependabot) and @kidinov (for Dependency Tree Diff) to help with this review in case I am missing something very obvious. PS: I even copy-pasted your testing instructions... 😅 But, on a more serious note, thank you both for all the work you did already on all that, thus making it so easy for me to follow your lead, you rock! 🪨

Related WCAndroid PRs:

I'll also publish a P2 post next week (see NEW) notifying WPAndroid developers about this change and providing more details about the process going forward, mainly (and again) copy-pasting the existing process that is already in place within WCAndroid.

Related P2s:

Android P2 - Surfacing Hidden Change to Pull Requests (dependencies) -> paqN3M-qo-p2
Woo Mobile - Automated code reviews assignment on Github -> p91TBi-4N0-p2
NEW: WordPress Mobile Apps: - HACK Week: Dependabot & Dependency Tree Diff -> pbArwn-41v-p2

To test (Dependabot):

It'll start working after merge. 🤷

To test (Dependency Tree Diff):

Try to add/remove/change the version of any dependency in the project.
Push the changes.
Wait for CI and notice a comment in this PR.
Revert your change.
Notice that comment is removed.

PS: You also can reference the below two test related draft PRs to verify correctness:

Regression Notes

Potential unintended areas of impact

N/A

What I did to test those areas of impact (or what existing automated tests I relied on)

N/A

What automated tests I added (or what prevented me from doing so)

N/A

PR submission checklist:

I have completed the Regression Notes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

This is a new 'ViolationCommentsToGitHubTask' that is going to be used alongside the dependency tree diff script to automatically add a PR comment with the dependency changes diff for reviewing purposes.

peril-wordpress-mobile · 2022-03-11T14:34:06Z

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

peril-wordpress-mobile · 2022-03-11T15:11:54Z

You can test the changes on this Pull Request by downloading the APKs:

Jetpack-pr-16092-build-119554.apk

WordPress-pr-16092-build-119554.apk

ParaskP7 · 2022-03-11T16:17:26Z

I am not quick sure why the Dependency Tree Diff job fails with the task not found exception, I tried a couple of things but nothing worked. Maybe this happens because the configuration needs to end up in trunk before the script can start working, due to the fact that we are switching to targetBranch at some point, I am not sure...

@kidinov any ideas? 🤔 🙏

FAILURE: Build failed with an exception.

* What went wrong:
Task 'dependencyTreeDiffCommentToGitHub' not found in root project 'project'.

kidinov · 2022-03-14T11:01:55Z

@ParaskP7

due to the fact that we are switching to targetBranch at some point

Yeah you are right

It works in the PR which is based on the current branch: #16096 (you can use this for testing too)

If you want to improve this, then just save the current branch in a variable and then add

git checkout "$currentBranch"

on line #45

wzieba · 2022-03-14T11:08:32Z

Dependabot config looks good to me! I'll keep an eye now on Autostattic 👀.

ParaskP7 · 2022-03-14T11:13:04Z

👋 @kidinov !

Yeah you are right

It works in the PR which is based on the current branch: #16096 (you can use this for testing too)

Thank you so much for double checking and actually testing this PR! 🙏 🥇 🚀

If you want to improve this, then just save the current branch in a variable and then add

git checkout "$currentBranch"

on line #45

👍

Btw, how would this improve the script if I add it there? Wouldn't the logic change if I check-out the current branch at this point, that is instead of using the target branch? 🤔

ParaskP7 · 2022-03-14T11:31:24Z

👋 @wzieba !

Dependabot config looks good to me! I'll keep an eye now on Autostattic 👀.

Thank you so much for the confirmation, and as always, for keeping an eye with Autostattic! 🙏 🥇 🚀

kidinov · 2022-03-14T12:20:40Z

@ParaskP7

Btw, how would this improve the script if I add it there? Wouldn't the logic change if I check-out the current branch at this point, that is instead of using the target branch? 🤔

It won't change much, except it will fix the case you have right now in this PR, when on the target branch there is not task dependencyTreeDiffCommentToGitHub and it will use this task from the current branch

wzieba

Looks good!

wzieba · 2022-03-18T17:57:56Z

.github/dependabot.yml

+      - dependency-name: "com.android.tools.build:gradle"
+      # Bumping 1.2.1 to 1.3.0 causes some issues, fist spotted in Reader. For more details, see
+      # https://github.com/wordpress-mobile/WordPress-Android/pull/14431
+      - dependency-name: "com.google.android.material:material"


I wouldn't recommend to ignore it - I believe sooner or later the bump will be necessary. Maybe it's worth to revisit and fix the issue?

Thanksor for the review @wzieba ! 🙏

I wouldn't recommend to ignore it - I believe sooner or later the bump will be necessary.

Yes, you are right. I just chose to ignore it for now and deal with it explicitly because last time this simple update happened, noone noticed it and it almost when live until we noticed that some layouts are having problems. Thus, I felt that if we don't ignore it for now, when the bot will generate the PR, someone will smoke test it and potentially approve it without doing a thorough test or knowing this wierd correlation between Material and Constraint Layout.

Maybe it's worth to revisit and fix the issue?

What if I creat a specific Material update issue explaining all about the problems coming with it, that Constraint Layout is related somehow and might also need to be updated as well. Then, I can keep this ignoring rule and add a new comment line with the linked issue that should address it. Wdyt? 🤔

Personally I'm ok with you creating a specific issue explaining all about the problem, as long as that issue does not end up in limbo and gets forgotten (maybe assigning it a Milestone to target it in a specific future sprint?)

I'd then recommend adding an additional comment on this line to point to said future issue.

On it! 💯

FYI: I created this accompanying #16132 issue for that matter, let me know what you think. 🙏

PS: I also added an extra link comment commit for that matter.

Thanks @ParaskP7 ! I'm going to merge this so it lands before I start the 19.5 code freeze 🎉
(We can always follow up with an additional PR if @wzieba still disagrees with it being ignored anyway and we decide to change it)

Great, thank you so much @AliSoftware ! 🚀 🟢

It's all good on my end, thanks for establishing strategy for handling this dependency!

AliSoftware · 2022-03-21T10:08:41Z

.github/dependabot.yml

+      - dependency-name: "com.android.tools.build:gradle"
+      # Bumping 1.2.1 to 1.3.0 causes some issues, fist spotted in Reader. For more details, see
+      # https://github.com/wordpress-mobile/WordPress-Android/pull/14431
+      - dependency-name: "com.google.android.material:material"


Personally I'm ok with you creating a specific issue explaining all about the problem, as long as that issue does not end up in limbo and gets forgotten (maybe assigning it a Milestone to target it in a specific future sprint?)

I'd then recommend adding an additional comment on this line to point to said future issue.

AliSoftware · 2022-03-21T10:10:34Z

.github/dependabot.yml

+      # Bumping 1.2.1 to 1.3.0 causes some issues, fist spotted in Reader. For more details, see
+      # https://github.com/wordpress-mobile/WordPress-Android/pull/14431
+      - dependency-name: "com.google.android.material:material"


Something like this, except be sure to update the link to the issue once you've created it.

Suggested change

# Bumping 1.2.1 to 1.3.0 causes some issues, fist spotted in Reader. For more details, see

# https://github.com/wordpress-mobile/WordPress-Android/pull/14431

- dependency-name: "com.google.android.material:material"

# Bumping 1.2.1 to 1.3.0 causes some issues, first spotted in Reader. For more details, see

# https://github.com/wordpress-mobile/WordPress-Android/pull/14431

# So we plan to handle the update manually later, to handle it in correlation with `Constraint Layout`; see

# https://github.com/wordpress-mobile/WordPress-Android/issues/<link-to-issue>

- dependency-name: "com.google.android.material:material"

peril-wordpress-mobile · 2022-03-21T10:38:19Z

	Warnings
⚠️	This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

ParaskP7 added 2 commits March 11, 2022 15:59

Tool: Add dependabot configuration file

a134a05

Build: Add dependency tree diff comment to github task

f30ca1d

This is a new 'ViolationCommentsToGitHubTask' that is going to be used alongside the dependency tree diff script to automatically add a PR comment with the dependency changes diff for reviewing purposes.

ParaskP7 added the Tooling label Mar 11, 2022

ParaskP7 added this to the 19.5 milestone Mar 11, 2022

ParaskP7 requested review from a team, kidinov and wzieba March 11, 2022 14:33

ParaskP7 self-assigned this Mar 11, 2022

ParaskP7 force-pushed the config/add-dependabot-and-dependency-tree-diff branch 2 times, most recently from bae1376 to 2aef057 Compare March 11, 2022 14:53

Tool: Add dependency tree diff script

83c9e99

ParaskP7 force-pushed the config/add-dependabot-and-dependency-tree-diff branch from 2aef057 to 9c9bae4 Compare March 11, 2022 15:31

CI: Add dependency tree diff job to circle ci

fde135e

ParaskP7 force-pushed the config/add-dependabot-and-dependency-tree-diff branch from 9c9bae4 to fde135e Compare March 11, 2022 15:34

Tool: Add current branch to dependency tree diff script

fcc85ec

ParaskP7 marked this pull request as ready for review March 14, 2022 16:13

This was referenced Mar 16, 2022

Test dependency tree diff #16096

Closed

Test dependency tree diff 2 #16100

Closed

wzieba approved these changes Mar 18, 2022

View reviewed changes

AliSoftware approved these changes Mar 21, 2022

View reviewed changes

ParaskP7 mentioned this pull request Mar 21, 2022

Update Material Components Library #16132

Closed

1 task

Docs: Add an extra link comment for the dependabot ignored material dep

51bf728

AliSoftware enabled auto-merge March 21, 2022 10:42

AliSoftware merged commit cff40dc into trunk Mar 21, 2022

AliSoftware deleted the config/add-dependabot-and-dependency-tree-diff branch March 21, 2022 10:51

Conversation

ParaskP7 commented Mar 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

To test (Dependabot):

To test (Dependency Tree Diff):

Regression Notes

Uh oh!

peril-wordpress-mobile bot commented Mar 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented Mar 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ParaskP7 commented Mar 11, 2022

Uh oh!

kidinov commented Mar 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wzieba commented Mar 14, 2022

Uh oh!

ParaskP7 commented Mar 14, 2022

Uh oh!

ParaskP7 commented Mar 14, 2022

Uh oh!

kidinov commented Mar 14, 2022

Uh oh!

wzieba left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AliSoftware Mar 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peril-wordpress-mobile bot commented Mar 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ParaskP7 commented Mar 11, 2022 •

edited

Loading

peril-wordpress-mobile bot commented Mar 11, 2022 •

edited

Loading

peril-wordpress-mobile bot commented Mar 11, 2022 •

edited

Loading

kidinov commented Mar 14, 2022 •

edited

Loading

AliSoftware Mar 21, 2022 •

edited

Loading