REQUEST: Promotion to Reviewer for @agilgur5

[agilgur5]

Follow-up to #221

GitHub Username

@agilgur5

Sub-project(s)

Promotion is requested for the following sub-project(s).

• Argo CD
• Argo Events
• Argo Rollouts
• Argo Workflows

Sub-sub-projects:

• argo-ui would be helpful as well if possible
• pkg too as it has some helpers that Workflows uses. But I haven't touched that as much as argo-ui

Requirements

• Member for at least 3 months (as of the membership meeting date)
• Reviewer for or author of at least 5 substantial PRs to the codebase

Selected PRs

• An upgrade of virtually the entire UI build system that was several years out-of-date: build(ui): upgrade to Webpack v5 + upgrade loaders + plugins argo-workflows#11628
  • This may not be that many LoC (outside of the lockfile), but was a gargantuan effort that required significant trial-and-error, finding lots of dated upgraded guides, etc. The PR description is long and full of links about this.
  • Fixed a pretty gnarly, subtle follow-up bug from this by using a super minimal, dependency free implementation of an FNV hash: fix(ui): don't use Buffer for FNV hash argo-workflows#11766
• Some very complicated UI refactors of some of the most complex components in the codebase: refactor(ui): convert WorkflowsList + WorkflowsFilter to functional components argo-workflows#11891, refactor(ui): migrate Reports to functional component and split files argo-workflows#11794
  • I've also spear-headed, completed, and reviewed nearly all of the UI hooks / functional components refactor, involving 30+ components (see my comments on the issue which link to many more PRs)
  • And completed removal of all explicitly deprecated components, some of which used 6+ year old APIs that pre-date the official React Context API
• Full copy-edits of multiple pages in the Docs: docs: full copy-edit of webHDFS page argo-workflows#11516, docs: full copy-edit of DAG page  argo-workflows#11943
  • I've added to many parts of the docs and have rewritten sizeable chunks. I have also significantly enhanced and fixed the docs contribution guidelines
• Fixed a pretty gnarly and subtle UI bug that caused dropped network requests: fix(ui): don't reload the page until _after_ deletion argo-workflows#11711
  • I suspected this myself during a code review of a PR (not due to the changes of the PR, but was an incidental, related finding), then successfully root caused it all the way down to a status code in the log in the issue. Did so after a request for assistance from @terrytangyuan. See detailed PR description et al for more details.
    • Not that it's bad to ask for help in any way, shape, or form! I mean that I've been helping out wherever I can and whenever asked 🙂
• Fixed two fairly popular argo-helm bugs by root causing them upstream to the Workflows Server itself: fix(server): don't grab SAs if SSO RBAC is not enabled argo-workflows#11426
  • Only a few LoC, but the root cause analysis took a solid amount of time to dive in and figure out what was going on
  • My first contribution to Workflows, feat(server): support name claim for RBAC SSO argo-workflows#10927, also required a substantial root cause analysis to determine. These both happen to be due to the Server's SSO logic
• Fixed some flakey tests that were occurring 50%+ of the time (anecdotally) on CI, significantly blocking PRs
  • These took quite some time to track down and pattern-match, and because they're E2Es, they were quite time-consuming to test as well.
  • This was a dramatic improvement to CI success rates to 90%+ (anecdotally) -- some of the most impactful 5 LoC I've written 😅
  • Made other significant QoL improvements to the developer experience on Workflows as well, such as ci: only run jobs when relevant files have been changed argo-workflows#12006, build: fix GH Codespaces building indefinitely argo-workflows#11981, build: replace kit with task argo-workflows#11928
• Did a substantial upgrade of argo-ui which required a lot of compatibility fixes and fairly sizeable debugging: fix: upgrade argo-ui components to latest argo-workflows#11585
  • Did so after request for assistance from @terrytangyuan. I debugged most of it in this comment and then fixed it all. See detailed comment and PR description for more details.
  • @terrytangyuan and @sarabala1979 then asked me to help with several other dependency upgrades that were failing CI. Some of these were also quite complicated to investigate and upgrade: chore(deps): upgrade monaco-editor to 0.30 argo-workflows#11593, chore(deps): update monaco-editor to latest 0.41.0 argo-workflows#11710, [Snyk] Upgrade react-monaco-editor from 0.50.1 to 0.54.0 argo-workflows#11598 (comment) (and further comments), [Snyk] Security upgrade swagger-ui-react from 4.12.0 to 4.16.1 argo-workflows#11640 (comment) + chore(deps): bump swagger-ui-react from 4.12.0 to 5.1.3 in /ui argo-workflows#11418 (comment)
    • Same as above, not that it's bad to ask for help in any way, shape, or form! I mean that I've been helping out wherever I can and whenever asked 🙂
    • Solo found and resolved several other dependency issues: chore(deps): remove unused JS deps argo-workflows#11630, ci(deps): dedupe yarn.lock, add check for dupes argo-workflows#11637, chore(deps): remove unneeded Yarn resolutions argo-workflows#11641
  • This also led me to take an interest in contributing to and improving the state of the fairly neglected argo-ui repo. See below in "Other" about stats for this repo and contributions I've made there

Selected Code Reviews

• A few detailed UI code reviews I did that went through several iterations: feat: show history about completed runs in each cron workflow argo-workflows#11811, feat: show history about completed runs in each cron workflow argo-workflows#11790, refactor(ui): workflow panel components from class to functional argo-workflows#11803
  • There were some very subtle bugs (e.g. global CSS issues, memoization issues) that I caught during these reviews too
  • Received much love from a first time ever OSS contributor too! ❤️ 🎉
  • Also caught a few missed anti-patterns and bugs in these UI PRs: fix: cron workflow initial filter value. Fixes #11685 argo-workflows#11686 (comment), fix: show pagination warning on all pages (fixes #11968) argo-workflows#11973 (comment)
• Several Server code reviews I did went through many iterations as well
  • This PR had 3 features/fixes in a single PR and had 30+ comments from me on it: feat: Add support for retrieving ClientId, ClientSecret and SSO secret from env var as opposed to creating a kube Secret argo-workflows#11777
  • Reviewed the filterSSOGroups feature in-depth as well as its docs: feat: filter sso groups based on regex argo-workflows#11774, feat: document usage of filterGroupsRegex argo-workflows#11778
    • I would say that some of my suggestions were important improvements, such as compiling and erroring out early, following the existing docs style, adding details about gotchas, etc.
      • Notably my in-depth reviews caught some missed side-effects to other parts of the codebase among other issues.
• Some Controller bugfixes that have went through multiple iterations after I caught some missed regressions during review: fix: refactor runConfigMapWatcher to use Informers. Fixes #11657 argo-workflows#11855, fix: controller watches global configMap in the namespace where it is running and not in managedNamespace - fixes #11463 argo-workflows#11799
• Many of my docs code reviews were fairly substantial rewrites of the original PR: docs: link Core Components to Architecture argo-workflows#11342 (review), docs: modify to detailed links + add details to running-locally argo-workflows#11405 (comment), docs: Add description about img in artifact-visualization.md argo-workflows#11732 (review), docs: Add installation option description argo-workflows#11735 (review)

Total

• 100+ PRs opened
• 215+ PRs responded to and/or reviewed
  • Have received much appreciation for my thorough reviews 🙂
  • See also Julie's comment below ❤️
• See issues, discussions, etc stats below in "Other"

Familiarity

• I am very familiar with the Docs, UI, CI + build systems, Server, CLI, and relatively familiar with the main Controller loop, but still have more to learn there, in the Executor, and some of the test suites.
  • I think I may know the Docs the best of any current contributor+ (if I may be so bold as to say), the UI build system the best (given I've rewritten and upgraded most of it), and large portions of the UI (similarly have refactored large parts of it).
    • Have reviewed most docs + UI PRs since becoming a Member, sometimes in great detail, finding bugs and requesting many changes
• I'm also a very experienced user, hence why I can contribute to docs and answer questions very well 😉
  • Also why I contribute to argo-helm occasionally
• I think most contributors know me too, in the other disambiguation of the word "familiarity" 😝

Other contributions and stats

• According to CNCF DevStats for Argo Workflows (take that with the same of grain of salt as any other metric), I am top 4 contributors in the past 3 years that DevStats tracks, top 2 in past year - 6 months, and top 1 in past 3 months - 1 month

  • Here's a screenshot at 2 years and at 6 months (I am in purple):
    
    

  • Notably, I have achieved that despite only actively contributing the past 4 months, i.e. in a much smaller time period than the width of the range, the entirety of which the average is taken from.

  • Also notably, I have largely not been paid by any company to make most of my contributions (maybe my first one could be considered under scope of my employer as it was a very specific feature for their SSO system)

• I've responded to hundreds of users over issues (220+), discussions (70+ + many sub-discussions in giscus), and Slack
  • re: Slack, can search me in #argo-workflows where I am very active, #argo-contributors where I am occasionally active, #argo-wf-contributors where I've weighed in as well as made bugfixes, reviews, and changes, and an occasional answer in #argo-helm-charts and #argo-events.
  • Have closed out and consolidated many issues, added new labels and label descriptions, significantly reorganized the usage of some existing labels and significantly labeled hundreds of issues, and made other maintenance improvements (such as recommending removing stalebot and Snyk automated PRs)
  • Helped several users in issues or Slack make their first contributions! 🎉
  • Encouraged some contributors to request membership as well, such as in REQUEST: New membership for @sonbui00 #249 and REQUEST: New membership for @shmruin #251 in this repo 🙌 🙂
  • Also helped making sure some useful (and sometimes stale) bugfixes got cherry-picked into patch releases
  • Also have been trying to put the lights back on on argo-ui (issues + PRs, Slack thread)
• Have come to every Workflows Contributor Meeting since I learned they existed as well as the Community Meetings and one Bug Triage meeting.
  • I've also done most of the Bug Triage independently on my own whenever I check on the issues after I learned the process (as @juliev0 can attest to).
    • EDIT: The Bug Triage meeting has now been cancelled since I've been doing triage myself near daily the past few months
• Come to SIG Scalability when I can (can also find me in #argo-sig-scalability)
  • Working on auto-scaling sharding for Workflows's Controller as well
• Come to SIG Security as well. I ran a security team at my last job, so fairly experienced in blue teaming InfraSec, AppSec, and ITSec
  • Working on SLSA Level 3+ and other OpenSSF Scorecard improvements for Workflows
• Submitted a CFP on various Multi-cluster Architectures for ArgoCon NA 2023. Currently waitlisted!
  • EDIT: may end up presenting this at a recorded Community Meeting instead

Misc Notes

I am also a very experienced maintainer, having maintained other repos since ~2014, which is why I'm pretty comfortable in the issues and with the general OSS process. Here are a few I maintain/previously maintained.
Contribute to various projects in the k8s ecosystem too, Argo obviously being the one I contribute to far more than any others at this point 😉

Lead Sponsor(s)

Promotion to Reviewer requires sponsorship from at least one sub-project Lead.

• @terrytangyuan

Other Sponsors

Listing some from the same sub-project:

• Approvers:
  • @juliev0
• Reviewers:
  • @isubasinghe
• Members:
  • @JPZ13
  • @caelan-io
  • @jmeridth
  • @toyamagu-2021
• probably more who haven't seen this yet too 🙂

[juliev0]

Happy to be a second sponsor!

[juliev0]

@agilgur5 probably gives the impression to many of the users writing issues and contributors writing PRs that he is one of the central maintainers of the project. The activity level on the project has probably doubled since he started becoming so active.

He's also looking at central issues within the project that have been problematic for awhile such as how to fix our flakey CI tests.

[JPZ13]

Happy to sponsor

[terrytangyuan]

+1 Happy to sponsor this!

[isubasinghe]

Happy to sponsor, @agilgur5 has been doing some great work. Really appreciate trying to understand bugs at their root cause.

[juliev0]

I might be wrong, but I just want to add that @agilgur5 might be the most thorough PR reviewer on the project from what I've seen (which to me is the most important criteria for promotion). These are for PRs across docs, UI, Controller, and Server in fact.

It’s clear that despite his not having worked on this project long, he’s been able to onboard himself fast, due to his strong expertise as a user (he knows how it should behave) and as somebody who has worked with K8S Controllers in the past (ArgoCD).

[sarabala1979]

+1 Happy to sponsor @agilgur5! Thanks, @agilgur5 for your contribution to Argo workflow