pluto_hdl_adi/docs/hdl_pr_process.md

6.1 KiB
Raw Blame History

HDL PR process

This page contains the pull-request process that the Analog Devices, Inc. HDL team follows.

Check out:

For first-timers

  • Make sure you are making a PR from the proper account (check First-Time Git Setup if you haven't already done so)
  • You will be asked by CLAssistant to "sign our Contributor License Agreement before we can accept your contribution."
  • If you see this message on your PR page: "username seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account." then it means you didn't check properly the first step
  • If you encounter conflicts with other files (that you didn't change, and that are already on master), do not resolve the conflict using Git GUI! This way, you will insert a merge commit into the commit history, and merge commits cannot be squashed with normal commits. We do not want merge commits. So, open a terminal and resolve it there (see this discussion)
  • Add the FPGA group as reviewers (analogdevicesinc/fpga group)

If you're the owner of the PR

Before opening a Pull Request:

  1. Rebase branch onto latest master
  2. Make sure the new register map is updated, if that's the case:
  3. Check if in the meantime there were any changes to the common IPs that you used (e.g., up_adc_common, up_adc_channel, up_delay_cntrl, etc.). If there are changes in the I/O ports, update your instances accordingly
  4. Regenerate the Makefiles that concern the projects you're editing/adding, to have them up-to-date
  5. Run check_guideline.py on your branch
  6. Run Verilator
  7. Visually inspect the code
  8. Code must build OK on at least one project. Warnings are reviewed. Critical Warnings are not accepted
  9. Test code in hardware on at least one setup
  10. Check README links

When opening the Pull Request:

  1. Give a detailed description for the PR (add link to related PR if it depends on others, maybe link to software PR, etc.), everything that is relevant for the reviewer
  2. In the description of the Pull Request, identify all links for Wiki where changes need to be reviewed, so that the Wiki approval and the HDL PR merge happen at the same time
  3. Add some labels to be easier for others to review your changes
  4. Check GitHub actions
  5. If reviewers requested changes or you found mistakes, then:
    • No force-pushing, even if there are tiny changes or typos
    • For every change, a new commit at least
    • Check the GitHub actions that are failing and fix the issues
    • Add a comment explaining what you modified additionally (it's easier for the reviewer and for tracking)
    • When the PR is approved by at least 2 people, you have 3 options
    • Option 1: If all these commits must be pushed, then from the dropdown, select Rebase and merge
    • Option 2: If all these commits must be in one commit in the end, then you can use the Squash and merge option from the dropdown. It will prompt you to give the name of the final commit
    • Option 3: Squash the commits locally, force-push and if you don't make any changes to the code, then GitHub will recognize this force-push as being without changes, so you don't need approves again to merge it using Rebase and merge
      • If you do make changes (don't!!), comment on what you did and ask those people that previously approved the PR, to approve it again changed files will be seen with Changes since last view next to the name (in the PR > Files changed tab)

If you're a reviewer of a PR

  1. Visually inspect the code and mark the viewed ones by ticking the box next to the name. In case the owner makes some other changes to the viewed files, you will see with Changes since last view next to the name (in the PR > Files changed tab)
  2. If the design is new, check the schematic and the pinout
  3. Check README links
  4. Build at least one of the affected projects and check warnings
  5. Make sure the new register map is updated, if that's the case. The IPs typically should follow Semantic Versioning 2.0.0
  6. Check if in the meantime there were some changes to the common IPs that were used in the project from this PR (e.g., up_adc_common, up_adc_channel, up_delay_cntrl, etc.)
  7. See if Makefiles are up-to-date by regenerating them (the ones that concern the edited projects)
  8. Run check_guideline.py
  9. Run Verilator
  10. Review the wiki changes
  11. Check the GitHub actions if they fail