Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: PR/atef

...

New ECS PD Campaign: Code Review

Where we are now

ECS currently provides nearly all of our hutch-python and related Python source code in the pcdshub organization on GitHub.

We utilize a standardized workflow wherein:

  • We have a canonical repository in the pcdshub organization.
  • We create forks of repositories in our individual GitHub accounts.
  • We create per-feature branches and open Pull Requests to the primary repository.
  • We describe, in detail, the changes we are making and the implications of those changes.
  • We ensure that our tests remain passing.
  • We review, check, and eventually approve each set of changes.

This offers several major benefits:

  • It allows for ECS engineers to more easily work together, using the standardized workflow described above.
  • Code is no longer just developed in isolation with a solo engineer's effort. There is a shared understanding of what is going on and why.
  • Being open to the public, it enables easy collaboration with our partner laboratories and other interested parties in the XFEL/synchrotron communities.
  • Continuous Integration - each of our repositories has a set of tests that ensure the code remains in the intended working state.
    • These tests are automatically run on every commit pushed to GitHub. It's like having an additional set of eyes on your code all the time, or an automatic reviewer of sorts.
  • We can automatically published documentation in an easily-accessible location on the web.
  • We can share tooling among all our projects.
  • We catch more issues and bugs earlier in the development process than we would otherwise.
  • We reduce maintenance efforts by a combination of all of the above.

Where we want to be

We want to take our same standards that we apply to Python development and bring it forward to all PLC, EPICS module and EPICS IOC development.

The goal here is not to just create Pull Requests but rather follow a code review process. We also need to be sure that an appropriate amount of effort is being spent on the process and not overlooked.

To reach that goal, we will need to make a concerted effort to:

  • Bring all of our EPICS modules and IOCs on GitHub
  • Ensure consistency between our local AFS-based repositories and those on GitHub
  • Work on continuous integration tools for:
    • Automatic linting of EPICS IOC process databases
    • Automatic EPICS IOC and module building
  • Take into account the social aspects of the change. We need to set expectations and ensure that both the submitter and reviewer are on the same page.
    • PR submitters need to respect the reviewers' time by providing enough context about the goals of changes and promptly responding to questions.
    • Code reviews can feel personal at times to the submitter, even if the reviewer is merely commenting on code.
    • A glance often isn't enough - reviewers need to give their due diligence to ensuring the changes are as described and without side-effects.

There are a number of standards to be defined and technical decisions to be made during our campaign. We are only just beginning with this, and any input would be appreciated.Ken Lauer Robert S. Tang-Kong 

ATEF Status Update

We are continuing on our refocused ATEF development effort, targeting passive testing of control system devices. The "passive" portion of this means that it will be fully-automated and non-intrusive (that is, it will not move your motors or otherwise directly cause a PV to change).

...