-
Notifications
You must be signed in to change notification settings - Fork 24
Process for contributing new code
Here's an overview of the process we follow to contribute new code to the sidewalk repo. As you write code, also make sure to follow our style guide. This is broadly the process that you will follow when you're ready to write code for a new ticket while using git through the command line (though you can do the same things through Github Desktop):
$ git checkout develop // always make sure to branch off of develop
$ get pull // make sure you have the most recent code from the repo
$ git checkout -b <new-branch-name> // see branch naming convention section below
$ // write some code
$ git status // see what you've changed; ignore changes to docker-compose.yml and .ivy2/ files
$ git diff <filename> // use on each file you edited to ensure you don't commit something you didn't mean to
$ git add <filename> // if the changes look good, git add "stages" the file for commit
$ git commit -m "descriptive message" // commit the changes to the files you staged to your local branch
$ git push origin <new-branch-name> // push your new branch to Github!
One problem that it may be worth mentioning at the top here: When you're ready to commit changes to your code, do not commit everything at once with git commit .
or git commit -a
. Use git status
to see which files have been edited, then use git diff <path-to-file>
to make sure that the only changes there are changes you intended to make. Then you can use git add <path-to-file>
to add that file for committing.
This is particularly important because you do not want to commit your local changes to the docker-compose.yml
, since that will post our API keys publicly, which means that we will need to regenerate the API key and everyone on the team will need to update their keys. When you run git status
you will also often see files in the .ivy2/cache
directory that have edits. You never want to commit those files (and the edits are usually just timestamps automatically being changed). It isn't dangerous to edit them, it just clogs up the git history :)
The production server, projectsidewalk.io, runs the main master
branch of the git repo which contains stable code. For adding new features, fixing issues and bugs, we have another branch called develop
. When you start to work on an issue, you need to create a branch for your work from the develop
branch (since it has the latest code).
To name your branch, follow this convention:
<git-issue-no>-<brief-description>
E.g. You are implementing a fix for this issue: https://github.com/ProjectSidewalk/SidewalkWebpage/issues/474
So the name of the branch would be: 474-admin-update-activities-table
This nomenclature helps to automatically add a label 'work-in-progress' to the issue you are working on when you publish the branch to the repo and provides a good descriptive branch name. Please refrain from including the '#' sign at the beginning of the branch name, because that can mess up command line autocomplete :)
To test out changes that will affect different users in different ways, you should test your code locally.
- To login as a normal user: Simply create an account and login via the "Sign in" button in the top right of the page.
- To login as a mechanical turk user, visit
localhost:9000/?referrer=mturk&hitId=h1&workerId=worker1&assignmentId=a1&minutes=60
instead oflocalhost:9000
. If the session you made for that user times out, you can just change theworkerId
,hitId
, andassignmentId
in the URL to start an account for a new turker. If you want another session with the same user, just change thehitId
andassignmentId
. - To login as a researcher/admin, you have to manually change your role id. To do this, open the
user_role
table in the database, and change therole_id
corresponding to youruser_id
to 3 (Researcher) or 4 (Administrator). These IDs are defined in therole
table.- To find your
user_id
, you can search through thesidewalk_user
table entries for your username.
- To find your
- If you are modifying the validation interface, you will want to test on mobile as well. To test on your mobile device, find you IP address using
ipconfig
. Then go to<ip_address>:9000
on your mobile device instead oflocalhost:9000
. - Does your code change or add any new text that users see? If so, post in the Github issue thread with all the changes/additions in English and ask for translations into Spanish so those can be added with your PR. Then add both the English and Spanish versions. You can find examples of how to do it in both HTML and JavaScript throughout the code, but it will looks something like this:
- If you are adding to the HTML, add a line to both
conf/messages.en
andconf/messages.es
with the translations, and reference those in the HTML file using@Messages("your.translation.id")
. - If you are adding to JavaScript, add the translations to the appropriate files in
public/locales
and reference them in the JavaScript usingi18next.t('your-translation-id')
.
- If you are adding to the HTML, add a line to both
- The last thing to do before creating a PR is to pull the most recent changes from on the develop branch into your own branch using
git pull origin develop
. Then test your changes on last time using this updated code, push that merge commit to your branch on Github, then create a PR.
Finally, when you submit a pull-request that will be reviewed before we merge it with develop
branch (and consequently to master
branch), write descriptive titles and include the issue number (e.g. #408: <PR Title>
) in the title. Finally, in the pull request description, mention the following things:
- "Resolves/Fixes
<#issue-number>
": Write either the words "Resolves" or "Fixes" along with the issue/bug number. It needs to be added when you create the PR. This will automatically add the "pull-request-submitted" label to the issue you are working on and to the pull request. For an example, see this PR. - Add a brief description of the main changes you made and if it would have any side-effects to other parts of the application. Again, refer to the previous example for reference. Additionally, it is always nice if you give instructions to other users on how to test your changes.
The automatic labeling ('work-in-progress' or 'pull-request-submitted') is done by waffebot that we use for managing issues. Hence, the requirements. Ask before closing an issue associated with a pull-request. The protocol is to test the fix on both the issue branch and develop
before closing the issue.
Things to keep in mind:
- Keep the PR small - only related to the issue in hand. Don't try to solve too many issues in one PR.
- Before submitting PRs for UI related issues, provide a before and after screenshot in the issue thread first. The team will provide their comments and we will come to a conclusion there. Then submit the PR. The PR thread should be reserved for all comments related to the implementation and things that come during testing.