Contributing (Connect apps)
When contributing a Connect app to the ShipEngine Connect platform, please follow the guidelines outlined in this document. Adherence to this set of standards will help to ensure that both the contributor as well as the reviewer have similar expectations related to the code contained herein.
Getting started
To get started with Connect, follow the online documentation to create a new app.
Once you have installed the Connect npm package =>
npm i @shipengine/connect -g
, you will be able to generate a new templated base repository via the cli =>connect init
.As a best practice to help ensure that visibility is clear for any changes made after the template has been scaffolded on your local machine, we ask that you create an initial commit before adding any custom changes.
# Scaffold the app.connect init...[answer the Connect generator questions]...# Initialize a git repo.git init# Stage the scaffolded file to be tracked by git.git add .# Commit the repo in its new, pristine state.git commit -m "Initial commit, scaffolding is now in place... no other changes."Open a pull request (PR) in GitHub at this point so we can get the baseline code in place to iterate from thereafter.
The repo is now ready for you to begin implementing your Connect application.
Best practices
General guidance
- The documentation site can be found here, http://connect-v1.shipengine.com/. This is the best single resource to answer your questions related to building a Connect app. Please familiarize yourself with these docs.
- It will be helpful to review the Sample Apps as well to get a grasp on how implementation can be handled in a few example scenarios.
- Do not use non-public shared packages, as they will fail to build and deploy properly inside our production environments.
- If a package is not publicly hosted and available via
npm
, it should be not referenced as a dependency within your app. - This is due to the fact that your app will be unable to reference anything that resides outside of that root application directory (where the package.json is found).
- If a package is not publicly hosted and available via
- Naming is important when defining the
npm
package... as this is used to map the application to the infrastructure that host's it.- Be sure to properly define the package name as well as the company name.
- Example, the
Acme
company is developing theShip-Right
carrier application.- The company name / scope should be:
@acme
- The application name should be:
ship-right
- The company name / scope should be:
- Example, the
- Be sure to properly define the package name as well as the company name.
- We publish updates frequently, so be sure to keep your
connect-sdk
dependency up to date. This ensures that you have a better development experience as well as a smoother PR review process.
Expectations for contractors
- Deliver a functional application that fulfills the outlined requirements within an established timeframe.
- As development progresses, status updates and communication of any barriers should happen without delay so they can be addressed in a timely manner.
- Clear communication is essential to delivering a high quality integration and should be done throughout the process.
- Feedback on how the offering can be improved to facilitate better development should be a primary consideration.
When the docs are not enough
- When questions arise which are not addressed within the documentation, they should be directed to the Engineering Coordinators to effectively answer the questions and work with the team to update the Connect SDK and/or documentation to clarify the situation.
- Likewise, when there are features or elements that are supported within a Carrier or Order Source which do not correlate with any available methods in ShipEngine Connect, we would like to be made aware of them so that we can investigate and potentially fill the noted gaps in supported functionality.
Code quality
Security is paramount. As such there is a lot of time and thought that should be taken to ensure both the code as well as the process is secure. Listed below are some common considerations that should be taken to help ensure security gaps are considered and mitigated during the development phase.
Never store credentials inside the git repository.
Frequently
client_id
andclient_secret
's are needed to allow an integration to authenticate with 3rd party apis. The secret should always remain secret.- When you have the option... You should not pass secrets as part of a URL. If the API you are calling requires this, you may not have a choice. But if you have an option to send via the headers, you should choose the header option.
- Note that when using HTTPS, even the query string will be protected, but the path is likely to still end up inside the logs.
- When you have the option... You should not pass secrets as part of a URL. If the API you are calling requires this, you may not have a choice. But if you have an option to send via the headers, you should choose the header option.
Audit your commit history to ensure you have not committed sensitive data into the version control history.
git grep SENSITIVE_DATA_STRING_HERE $(git rev-list --all)
- This will search through your git history attempting to find instances of the sensitive data string.
- Should you get matching results, you should be sure to modify the git history to remove the values.
- This will search through your git history attempting to find instances of the sensitive data string.
Secure access to the git repo.
- Require 2FA.
- Never use credentials that are shared across multiple users.
- Contributors should only have access to the data they need to do their work.
- GitHub accounts are commonly personal accounts... be sure to remove access for users when they leave the company.
Do not make assumptions in code that could be hard to change later.
Code style
We suggest that you use the latest ES6 conventions. Please see our coding style conventions document here.
Commits
- Be sure your git config is properly setup... This will help to ensure that changes can be accuratley be traced back to the author who made the change.
- Work inside branches... do not commit code directly to
master
. - Each and every commit message should contain a meaningful description of the changes made within that commit.
- Bad:
Refactored entire app as requested.
- Good:
Updated yarn version from '1.17.0' to version '1.22.10', so we no longer get errors when node has not been installed as root.
- Note that a good commit message should describe both what was changed, as well as why the change was needed.
- Bad:
- Break up the commits into logical chunks. Each commit should be self contained, such that a single commit can be reverted and it will revert all that is needed to get back to a functional state.
- For example, if a dependency is updated which required code changes to take into use... You should include the update as well as the changes needed to make it function.
- These changes can be included within a single commit if the PR contains more changes than just this individual item.
- Or alternatively in seperate commits, with both inside a PR related to this specific change.
- Basically, commit related work together.
- For example, if a dependency is updated which required code changes to take into use... You should include the update as well as the changes needed to make it function.
- Do not re-write the history of published branches, like
release
/master
.- Git rebasing is fine for your topic branch, but not for published branches, like
release
/master
branches.
- Git rebasing is fine for your topic branch, but not for published branches, like
- Keep your working branch up-to-date with
master
. When working inside a long lived topic branch, it is easy to find that your branch has gone out-of-date with themaster
or branch you originally based from.- Writing and testing new code against a branch that is out of date is not useful since you cannot be sure that it will function properly once merged.
- You should frequently rebase against or merge the upstream branch into your local branch often.
Pull Request Process
README.md
- Always include a readme file that clearly describes the requirements of the integration, as well as how to get the app up and running after cloning it from the git repository.- README sections should include but are not limited to:
- Link to the website of the company being integrated with.
- Links to the relevant documentation.
- Include any relevant documentation files in a
/docs
directory when possible. - Descriptions of the supported
- Services
- Supported label types
- Application ID
- README sections should include but are not limited to:
- Partners are responsible for peer review iterations of their own modules prior to requesting a review from ShipEngine.
- Make those efforts transparent via Github.
- This keeps our engineering out of the minutiae of code quality, and other obvious technical issues.
- When creating a Pull Request (PR), you should be very clear and concise in both the
Title
as well as theSummary / Description
.- A good title will breifly describe what the PR is intended to do or change.
- A good description will provide a concise breakdown of what was changed and why... In addition any additional reference information and context should be linked or explicitly provided inside the PR summary.
- A PR should not be huge. Much like well designed code, a well made PR should adhere to a single responsibility principal. Multiple PRs with a clear seperation of responsibilities between each are far better than a huge PR with multiple changes with varying purposes throughout.
- Prefer multiple PRs with a single responsibility instead of a single PR with the whole app's implementation for review.
- Once review comments have been made on a PR:
- For suggested changes create, accepted, and commited via the GitHub Suggested Changes Feature... once you have accepted and commited the suggested changes, the converstation will automatically be marked as resolved in the PR.
- IE, Rename method
getForDisplay
togetRecipientNameForDisplay
.
- IE, Rename method
- For more general requested changes, you should make the changes and await or request a re-review. Once the reviewer has signed off on the changes, the reviewer will mark the conversation as resolved.
- IE, Refactor the api call to use the XML endpoint instead of the JSON endpoint when calling the API... there are know issues with the JSON endpoint.
- For suggested changes create, accepted, and commited via the GitHub Suggested Changes Feature... once you have accepted and commited the suggested changes, the converstation will automatically be marked as resolved in the PR.