[kcdc 2022] lessons from reviewing a very big pull request

Speaker: Patrick McVeety-Mill @pmcvtm and @loudandabrasive

For more, see the table of contents

PR Principles

PR is to merge code. Typically involves gate checks and review

  • well organized – code resonably grouped, scope is defined and right sized, mostly related changes (not so small that annoying, not so big it is difficult to review. keep side stuff to a minimum)
  • well documented – high level description, story of the approach, issue/ticket/doc reference
  • well considered – planned before opening, open for discussion pragmatic not dogmatic
  • well reviewed – thorough (high and low level), ode efficient and matches style, tested (functionality over all else)
  • have empathy – for author, reviewers, future readers of code, users of product

Context

  • Open source software provider
  • This PR came from a partner – large changes in the past with reasonable success. Semi-rotating cast of devs
  • This change wasn’t made by the same people who learned how to do things first ti
  • Semi-collaborative – weekly syncs, shared chat room
  • Initial plan was for smaller chunks but hard to enforce
  • Doing free work for company because partner
  • Wound up being 900 lines

How approach looking at

  • Accept your fate – mentally prepare
  • Assess the scene
    • look at high evel and plan process
    • web UI doesn’t help with hundreds of files git diff —name-status main gives a list of the files and status. Redirecting to a file gives all of them
    • Dump into a spreadsheet – can sort by change type (add/modify/delete/rename, project or file name)
  • Translate into a checklist – track as go through files. Can do in markdown or Excel. Markdown lets you copy/paste into the response later
  • Look for smells – need to look at closely
    • R100 – means rename an no differences. (100% same.) Can be irectory rename or file name refactoring
    • Add/delete with similar names. Changed a lot.
    • Big chunks of adds – new features
    • ”C” s for copcying a file
    • At this point, leave feedback and ask to rename back to cut down the diff and can restore them in a follow up. This will make PR smaller
    • This got 900 file PR down to a 600 file PR
  • Disappear and review – look at each project/file. add/delete then changes.
    • git difftool -d main MyDir
    • Used BeyondCompare to see visual
    • Looking for critical differences
    • Quicky note format/style issues
    • Manually test related features before moving on
    • Don’t get stuck viewing diffs – want to understand big picture
    • Use diff tool most familiar with so not learning new tool while doing this. Your IDE may have a useful tool
    • If multiple commits, read comments to see journey
    • Use an IDE or tool that can highlight warnings – ex: unused code
    • track and note comments in your checklist
  • Give feedback
    • Be incremental – don’t wait til the end. Give feedback file by file/project by project. Can do in description with multiple posts.
    • Be specific and actionable list files/line numbers, allow author to indicate resolution, explain reasoning where can
    • For small PRs, comment inline during review (ex: GitHub). For super large PRs, crashed tab because so large. Gives top evel coment but reference the specific file names locations.
    • Prioritize with MoSCow – MUST do before merging, SHOULD do or create debt (task for later), COULD do to be nice, WON’T do even i we like (maybe an epic later). Also questions and comments.
    • Indicate nits. Still fix but clear what it is.
    • Compare against your notes to mae sure done
    • Saw ”when” – give tricky issues back to internal team. Can use epic/release branch. Can merge into there so not in main until fix everything.
    • Be diplomatic – remember everyone can see and don’t want to scare contributors away (free work, want to keep)
    • Don’t close PRs without comment

Timing

  • 1 month to merge to feature branch (1-2 weeks for first pass)
  • 2 months to finish all follow up issues

Preventative Measures

  • Formatting
    • pick a format and stick with it
    • setup auto-formatting
    • if strong arm, know it is annoying
    • report early rather than every instance
    • decide how important it is
    • .editorconfig files are cross language and cross-IDE
    • githooks
  • Repository Templates – placeholder text for PRs and issues to show what want
  • Automate checks and report back. Builds are for when code changes. Bots check things as time passes

Communication channels

  • PR interface good for tracking progress, leaving artifacts for others to look at, the original comments from squashing comits
  • PR interface not good for feature details, complex scenarios, brainstorming
  • Use shared issue tracking – just knowing the github issue is in progress for months isn’t helpful. Use a github board or trello
  • Get agreement on what PR will be
  • DIscuss offine and post result
  • Alternatives – Frequent pairing (still do PR at end), periodic review (ex: review monthly ”state of the union on the repo”), trust/testing/recovery (everyone merge to main)

My take

This was fun. It was very information heavy, but easy to follow. And I learned a few new techiques.

[kcdc 2022] level up with co-pilot

Speaker: Rizel Scarlett @blackgirlbytes

For more, see the table of contents

Notes

  • AI pair programmer
  • Not magic
  • Compare to Gmail smart compose – suggests continuations
  • Draws context from comments/code
  • Suggests lines/functions

Open AI

  • Powered by Open AI Codex – translates natural language into code
  • GPT-3 – generative pre-trained tranformer 3 – deep learning to produce human like text
  • Duolingo GPT-3 uses for grammar correction
  • Codex code based
  • Played with https://beta.openai.com/examples – Movie to Emoji and Mood to Color (many others).
  • There is also https://beta.openai.com/playground (if you login) where it can generate code from a comment. Even lets you specify a language
  • Also learned you can paste a hex code into google and have it show you the color

Copilot Labs

  • Hover over suggestion to get more
  • Experimental feature to include an explanation of what code does
  • Plugins – VS Code, JetBrains, Neovim
  • Can choose not to include public code
  • Can do some (human) language translation

Benefits

  • Code faster and clear – good at patterns, syntax (ex: regex) so don’t need to google, write better comments so copilot can give good suggestions
  • Write good docs – in markup
  • Be a better mentor – avoids nervousness of someone watching you type because don’t have to worry about syntax, helps mentor people in languages don’t know
  • Gain context for new concepts – studying for interviews (leetcode), explain new code base, create short demos in new languages as dev advocate

Tips

  • Turn off when writing initial structure. Turn on once have pattern going. Comments not useful at first.
  • Good when writing unit tests.

Cost

  • $10/month
  • Free if open source or student

My take

Rizel has a lot of energy and is very relatable. She also did “group play” with openai early. All of that helped engage the audience. I’ve read about co-pilot but it was really cool to see it and the features/benefits/use cases. I enjoyed seeing her passion for the tool and the examples. I also liked how she avoided it from devolving into an argument about the ethics of co-pilot. Rizel didn’t let the wifi problem throw her. It was unfortunate that the demo didn’t work even though other internet stuff did. [block? too bandwidth heavy?] The code to tweet was cool

For co-pilot, some looks cool. Some of the comments were longer than the code. So in real life, I imagine you wouldn’t use it for everything.

[kcdc 2022] building rugged devops pipelines with github

Speaker: Brian Gorman @blgorman

Repo: https://github.com/blgorman/codemash-rugged-devops

For more, see the table of contents

DevOps

  • Process, not product
  • Can’t buy tool (but can buy an existing team)
  • Goal: reduce cycle time form idea to production with minimal error
  • Automated testing
  • Gates – automated and human, quality, release
  • Avoid 2am support calls. Or at least only have to push a button to recover

Shift left

  • Less cost if find bug early
  • Reusable processes
  • Push quality upstream
  • Dev machine is as far left as can go
  • Build scanning tools into process
  • Sell the vision

Prereq

  • Cloud env exists
  • Templates

Actions

  • .github/workflows – folder
  • Actions tab to see result

YAML

  • on: the triggers (ex: push, pull_request)
  • env: (ex: branch)
  • jobs: want you want to do
  • with: trigger another job

Infrastructure as Code

  • Declarative
  • Repeatable Results
  • Ensures no configuration drift
  • Azure has imperative and complete options for ARM templates – complete is destructive and dangerous. Anything added outside code deleted.
  • Tools – ARM templates (Bicep) , Terraform, Ansible, Puppet/Chef, Pulumi

Security

  • App registration so github actions get access to Azure and add credential
  • Setup github token

SonarCloud Scanner

  • Most popular
  • Not free
  • Can choose whether to require fixing of warnings

Dependabot

  • Brining in dependencies without knowing
  • Alerts on insecure package
  • Options for security updates

OWASP

  • Zap Scan
  • Baseline (lightweight) scan for pull requests
  • Full scan overnight
  • For penetration test, provide valid URL so can try to hit it
  • WhiteSource – GP Security Scan

Q&A

  • Azure DevOps pipelines easier if new team
  • Azure DevOps has better gating
  • GitHub getting better.

My take

The room was full. When I walked in, Brian was talking. I was worried he started early, but did not. He was just talking to the audience who arrived early separate from the session. It was a good intro to github actions. Also if it has been a while (more features now.) I’m glad the code was in a repo so I could read it on my screen. I’m sitting in the back (because my talk is right after this and I need to leave early) and can’t read the code from here. It’s also nice to have as a reference. I also like that he covered integrations like Sonar.