Creating Pull Requests

While developing your code, we suggest you do the following:

• Create a branch of development branch (the main OED branch that is the default when you clone OED) and give it a name that indicates the work you intend to do. One can do this after the work is complete if not done earlier. Reusing a branch can cause unexpected updates to a pull request.
• Write test cases for your code. We use Mocha/Chai for testing and a description of its location and use in OED is available. We need to have all code carefully tested to know it is working when created but also stays working as future changes are made to OED. Writing good test code takes real effort but it needs to be done. If not done then that should be stated as a limitation of any pull request.
• Make sure you run OED in a web browser and it works as expected. Please check any feature that might have been impacted by your work and do a quick general check of the system so you know it works.
• Consider if your changes impacted code or documentation that needs updating. For example, did your code change OED usage so the documentation pages needed updating? Did you add a script so the Code Organization web page needs update? Did you change database usage or a function used elsewhere in OED so other code update is needed? It is impossible to give a full list but checking this out helps the project. In some cases you can note the needed changes in your pull request to make sure they get done outside your work.
• Run our checks. Lint is a system that verifies that your code does not have certain syntax or running issues along with making sure it follows our expectations for code style. It also verifies that each file has the needed license at the top and that TypeScript is used where expected. You will likely find it easier if you run this check early to find any issues so you don't repeat them many times (those working in VSC should see some being tested as your are coding). The checks are run in a terminal in the web/vsc Docker container:
  npm run check
  
• Run our code tests. These run all the test cases to verify the code is running correctly. It is similar to running lint where you use:
  npm run test
  This can take several minutes. A summary will be given at the bottom. If you see any errors then they should be checked out. Let us know if you need help. You will do the same command to check any new tests you add. See other info for faster running of single tests for debugging. Also note that the tests deliberately try to do invalid operations to test OED. This causes warning/errors in the OED output as well as in the PostgreSQL log that will be shown in the next install.
• Make sure you keep your fork up to date with development on the main OED GitHub. Other people may integrate new code into OED and your work needs to be in sync with that work.

When your fork has code that is ready to add to the OED code base, create a pull request from your branch into development on the main OED GitHub repository (GitHub documentation). The pull request template should include the following:

• An area at the top to describe this PR. It states: "Please include a summary of the change and which issue is touched on. Please also include relevant motivation and context." This is also the area to add recognition of others that worked on code. Use the @<GitHub name> where you put in the person's actual GitHub name (user id) without the angle brackets and GitHub will automatically link to their GitHub account.
• Note any issues this pull request addresses. Check to see if you are addressing any open issues. If so, please note in the pull request description. If your pull request completely fixes/addresses an open issue then put the words "Fixes #XXX", where XXX is the issue number. If it only partly addresses an issue then use "Partly Addresses #XXX". If it fixes an open issue then everyone will see a note in the pull request (after opened) indicating it will close this issue when merged and there is also a related note in the issue.
• There is a checkbox to indicate: "Note merging this changes the database configuration." This indicates that the database needs to be updated for these changes to work correctly. Normally there will be migration files to update the database. However, many developers will choose to reinitialize the database to do a clean install to be sure everything works correctly.
• There is a checkbox to indicate: "This change requires a documentation update". This indicates that some documentation (documentation pages, developer or website) needs modification based on the changes in the pull request.
• There is a checkbox to indicate: "I have followed the [OED pull request](../pr/) ideas"
• There is a checkbox to indicate: "I have removed text in ( ) from the issue request". The template puts directions in ( ) and they should be removed once done to save space and make the pull request cleaner.
• There is a checkbox to indicate: "You acknowledge that every person contributing to this work has signed the OED Contributing License Agreement and each author is listed in the Description section."
• An area to list any limitations of the PR: "Describe any issues that remain or work that should still be done."
• If your work is not yet ready and you are sharing to get early feedback, please mark the pull request as a draft.

When your pull request is submitted, automated GitHub Actions will run including static code checks and the OED test code that will do the checks and tests. If they pass then a green check mark will be put next to them and your pull request can be considered. If they fail, you should see why and fix the issue on your branch. When you push the change to your GitHub account (with the fork of OED), your pull request will automatically be updated and recheck. Note that the tests will not automatically run if this is your first pull request to OED. In that case it will wait until the project authorizes it to run.

You can tell if there were any issues with the automated GitHub Actions by looking at the commit on the pull request. Toward the right side of the line with the commit there will either be a green check mark (everything is okay) or a red x (indicating issues). (It could be yellow of the checks have not finished which normally takes 1-4 minutes.) If you click on either, a popup has a link to the "details". Normally the issue is with the OED tests (and not from the automated testing setup). To see these, open up the section labeled "node tests". This will show all the checks (lint, typescript, etc.) and the automated unit tests. Normally there will be an error message (either at the end or in several places). This should indicate where the error occurred and help in tracking it down. Note that an error can cause the testing to stop so it is possible there are more errors that will show up once the earlier one is fixed. See above about running the tests on your machine and the developer details page has information on running a limited number of tests.

Once your pull request is passing all automated checks, it needs to be reviewed by at least one OED team member with that privilege. The review might make suggestions and may request changes. Once that process is complete, a senior member of the team will either approve the pull request or make other comments (if not already reviewed by a senior member). Once approved, the pull request can be merged into development for inclusion into the OED code base.

As always, if you have questions about this process or run into issues, you can contact the development team using the link at the bottom of this page.