Development GitHub Workflow - wb8tyw/D-Rats GitHub Wiki

Development GitHub Workflow

Git hub has many features that are usable to help organize a project, and there is a lot to learn about it.

There are also many how-to articles on learning to use it.

If multiple users are working on a project is important that they are in sync with a common set of rules.

A GitHub Organization project allows the administrators of the project to limit what groups of members can do with the organization and various features.

A personal GitHub repository does not have any of those controls, and gives collaborators almost the same access as the owner, so it is up to the collaborators to limit what they are doing.

Note to Microsoft Windows Users

Microsoft has created their own ecosystem and default ways of doing things are not always compatible with the rest of the internet.

In general many file formats like ".rtf", ".doc*" are generally only used on Microsoft systems, and in general e-mail communications with projects are expected to be formatted in plain text unless the project management states otherwise.

Users on other platforms may not have readers installed to read Microsoft specific document formats, such as RTF format or some of many formats that Microsoft Office will create by default, so some attachments may not be readable or be harder to read for other platforms.

Many mailing lists and e-mail clients will attempt to convert HTML to plain-text automatically. This conversion is never perfect, and content from your message may be lost with out you realizing it.

And many developers will have HTML rendering disabled by default on their e-mail clients, partially because plain text can be easier to read, and also because many security exploits have occurred with HTML e-mail rendering programs in the past where systems have been infected by just opening an specially crafted message.

Unfortunately the last time I checked, and I will admit it has been a few years, Microsoft e-mail clients could not be set to send actual plain-text. When set to plain-text, they send what is known as "Quoted Printable" instead, which while it is all ASCII, many things in the message will be replaces an equals sign and two hex digits, which makes it hard to read at times.

Generally you may want to use a different e-mail client like Thunderbird for communication with open source projects.

Thunderbird can be configured to send a variant of plain text known as "format-flowed", which while it also has its problems but not nearly as bad as the Microsoft e-mail clients.

Many tools for text on a Microsoft platform will use by default a format where each line ends with both a "Carriage Return" and a "Newline". This shows up poorly on many applications and will cause problems in Pull requests or git commits.

In general text files are expected to be ASCII or UTF-8, and have each line end with a "Newline" character, otherwise known as a linefeed. Most Microsoft applications can read those formats with no problem. Older versions of "edit" and "notepad" can not read them. The program known as "write" or "wordpad" depending on which version of Microsoft windows can read these files with out a problem.

Applications like "Visual Studio Code" of "Programmer's Notepad 2" can also be set to use what they call Unix line endings by default.

Versions of git for Microsoft windows may need special settings to for them to be compatible with cross platform get repositories.

General best practices

  • Check to see if there is a Wiki for use of the repository that describes the desired policies.

Tickets

If adding Tickets is enabled, then you can file tickets about new issues or update existing issues. You can add your self to a ticket to monitor progress on the ticket.

It is best to use the web interface for updating a ticket. E-mails replies to tickets will update them, but the resulting contents may get mangled due to various incompatibilities. Assume e-mail attachments will be stripped.

In general once a ticket is opened then it should normally closed only by the repository maintainers after they have verified that it is fixed, or are unable to get feedback in a reasonable amount of time.

The originator of a ticket may usually also close a ticket if they determine that they opened the ticket by mistake and no work needs to be done or the ticket is a duplicate, unless there is a local policy that only the repository maintainers should close tickets. GitHub does not enforce this.

Tickets are usually for one specific issue, and may have links to related issues.

A ticket reporting a bug really needs a way to reproduce the bug, and if possible some log message indicating where the code may be broken.

A Ticket may also be for a larger task with steps to be tracked by individual specific tickets.

Updates to a ticket should only be to the specific issue that a ticket is covering, not used to add reports of unrelated bugs.

Tickets may have additional data for sorting as and tracking as labels, and there will usually be a repository specific convention on that to learn.

Unless it is the same exact issue as a closed ticket, a new ticket should be opened instead of reopening a ticket.

The repository management may change bug titles and labels as needed while processing the ticket.

Pull Requests

Pull Requests are proposed changes to be merged to a branch. Generally all changes will go through a Pull Request.

Pull requests for a bug fix should be linked to a ticket, describing the bug or enhancement.

  • Comments to Pull Requests should be limited to what changes that the pull request is making.

  • It is best to use the web interface for commenting on a Pull Request. You can make multiple comments on the changed code that way with one review, which will result in one e-mail. While E-mails replies to Pull request will also update them, due to various incompatibilities with e-mail, formatting can be mangled or lost. Assume attachments will be removed.

  • Related small enhancement requests to what an open Pull Request is handling can also be relevant, but it will be up to the author to if they want to add them.

  • Submitting updates to someone else's Pull Request can cause issues with them keeping their own development environment up to date. Do not do that unless you get permission from the Pull request's author. While you may know how to clean up the mess it makes to their local branch, do not assume that they know how to or want to do that.

  • Note that some Pull Requests are just refactoring, they should not change existing bugs. This should be noted in either the commit message or in a comment added to the Pull request. Please check for that before adding comments that something is still not working after the change that was not working before.

  • A merged or closed Pull Requests generally should not get additional comments, unless it is clear that merging of the Pull Request introduced a new bug.

  • Bug specific comments should go into tickets, not the Pull Request.

  • Use a fork of the upstream repository.

  • Never commit to the master branch directly after the initial commit to create the repository.

  • Git has its own general assumptions about the commit message and generally the vim editor will let you know if you are not in compliance by highlighting the violation.

See git-scm.com Distributed-Git-Contributing-to-a-Project for some strong recommendation for formatting a commit message.

  • Make a note of the commit message format used by the upstream.

I prefer to have a commit message list which files were changed in the commit and a brief description of what the change was under them.

There are tools to create change logs and release notes from the commit comments.

Python packaging is now stating that the Towncrier be used to maintain the NEWS.rst file. Towncrier does read the git commit comments to create its content.

It is possible that in the future, we can create a script to create the Towncrier files as needed.

Note that the Towncrier tool to create the files in the changes directory does not need to be used. It just creates an empty file with the name given. The Towncrier tool only needs to be run as part of packaging a release.

Some other links:

Conventional Commits v1.0.0

Mokkapps.de blog on how to automatically generate a helpful changelog from your git commit messages/

Keeping fork up to date with upstream

Do not merge your development branches into any branch that is copied from what was maintained by the master.

This means that you can use a force push to make those branches in your fork to be identical to the upstream.

The GITHUB web UI will have a button to create a PR request to update your repository. If you do that the branches will have the PR in addition to the updates so will not be the same.

See Working with forks on the steps for managing fork from the command line.

Automation tools like Jenkins can be set to monitor the upstream for changes and automatically update the fork.

Triggering an such an update on a change to the upstream generally requires the administrator of the upstream to setup a web hook, which contains a URL to a web server that is visible on the public Internet.

For a project with a low change rate, a simpler poll of a few times a day should be sufficient for automation.

The gitconfig File

This is assuming a Linux compatible git client.

One feature of linux compatible is to using Unix line endings, of a single line-feed.

You need the a minimum of the following in the .gitconfig file in your home directory

Todo, add the git commands that setup the file. I just normally use a text editor.

[user]
	email = [email protected]
	name = YourName Here
[core]
	editor = vim
[push]
	default = simple

Creation of a Pull Request

  • Create a branch in git for the change. If a ticket is involved, it may be useful to put the ticket in the branch name.

  • Make the edits and test them. Make sure that you run lint checks.

  • Commit the change to the branch.

  • Push the change to the branch on your local fork.

At this point ideally some automation will be run to do linting and some testing on the code.

For security, you should only allow automation to run code in pull requests from trusted contributors.

It generally takes an initial force push to create a branch for pull request.

Note, once a Human review has started on a Pull Request, you should not do a force push to the branch used for it.

Until a Human review has started on a Pull Request, you can update it with a force push.

If you need to experiment after a human review has started, use a new branch to test your changes and then when they work the way that you want, merge that change with the Pull request as one commit.

Tickets to my local fork

All feedback is welcome. Please try to describe if possible how to best to reproduce the issue in the case of a bug.

Fixes are only being done to my experimental fork and are not being also done to the official upstream of d-rats by me.

Priority is to fixing broken items in my fork over adding new features.

I have labels for setting the status and class of tickets. Important ones are "Waiting Verification" and "Verified"

A ticket waiting for verification is waiting for someone other than me to confirm that it is fixed. Then the "Verified" label should be put on it, and the "Waiting Verification" label removed.

I would be prefer to be the one that closes tickets, even though it may look like the work is done, there may be a reason to keep the ticket open.

Pull requests to my local fork

This fork is intended for my experimentation. Unless your PR is specific to something that I am currently testing, it is probably better to submit it to the upstream repository.

The python_tyw branch is the branch that I am maintaining for my fork.

The master branch is tracking the https://github.com/maurizioandreotti/D-Rats and is periodically synced to it, so any changes made directly to it that are not in Maurizio's repo will be erased.

Also I am trying to maintain my fork in to close to the same standards that I have been held to on professional standards. Some of this stuff is new to me, and some is stuff that I am trying to remember, so please bear with me.

This is also practice for working things out for Planing for a D-Rats Organization Project

Merging or landing a Pull Requests

I am the only one that should merging or landing a Pull Request to my repository even through people added to the project also get the ability to merge PRs, please do not merge any Pull Requests, even if you think they are complete.

I may merge pull requests with out any reviews on them, as I like to have only one pull request open at a time.

Commit messages

I would like the commit messages to be able to be automatically scanned to create a changelog per the general practices above. As I have not yet tested doing so, the format below may be changed in the future, once I get the process running.

See: https://www.conventionalcommits.org/en/v1.0.0/

I think the title line must be less than 68 characters, but need to look up the exact number. I currently rely on VIM to tell me when the line is too long.

There must be a blank like after the title line.

Lines should not be longer than about 70 characters, at some point I need to look up the reference for that.

I do not have a list of sections yet. In general it is either module name with out the file type for the majority of the changes, or the directory name that most of the modules are in. Example "(map)" "(config)"

fix:(section) Summary of change

More detailed explanation of change.
Issue number if there is an issue filed for it.

dir/filenme.py:
  Any global changes
  in module foo, make sure bar is of bytes type

DocStrings

All new classes/functions/methods should have DocStrings in the following the Sphinx format:

https://sphinx-rtd-tutorial.readthedocs.io/en/latest/docstrings.html

And some great help from what have not found else where.

https://samnicholls.net/2016/06/15/how-to-sphinx-readthedocs/

It is nice to add DocStrings when you can to changed code.

If you are mostly fixing DoCstrings or fixing pylint noise, then it is better for that to be in a separate pull request with no other coding changes. Just a guideline to help reviewers.

If you are changing actual code and DocStrings/pylint at the same time, reviewers might miss new bugs accidentally introduced into the code.

With recent commits to my fork the HTML documentation can now be easily generated.

You need the python-sphinx package to be installed.

The HTML document tree in the docs/_build directory can be generated with:

cd docs
make html

Currently the make procedure is generating warnings as I just got it running, and of course it is finding bugs in the existing doc-strings and modules.

Short term goal is not to add any more warnings. Long term goal is get all the warning messages to be resolved and to add additional helpful documentation. And eventually have this published by a github action.

You should be able use file:///absolute/path/to/d-rats/source/docs/_build from a web browser to inspect the generated documentation.

All Pull Requests should be scanned by pylint first.

PRs should have pass a pylint scan before submission.

Pylint is a tool that scans programs for errors and style. It has found many existing bugs in d-rats so far. Some of the style checks can be customized to the project, but in general I am trying to leave most of them at the defaults for now.

Even better if they are also scanned by pyright also known as pylance.

In general we want to fix all pylint messages possible. Unfortunately some pylint diagnostics can not be easily fixed at this time and must be suppressed with a line similar to the one below, with the messages below. # pylint: disable=message-one, message-two

Pylint suppressions are currently acceptable for otherwise unchanged code if unrelated to the change.

New code should pass pylint as much as practical.

If a lot of pylint changes are needed, it is better to split up into two Pull Requests, with the first Pull Request just being the changes silence the pylint messages. As with Docstrings, this is just a guideline.

See https://github.com/wb8tyw/D-Rats/wiki/Conversion-to-Python3-Notes#pylint

Automated tests on pull requests

Submitting Pull requests to upstream

See Creating a pull request from a fork