Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CONTRIBUTING.md. #464

Merged
merged 18 commits into from
May 14, 2024
Merged

Add CONTRIBUTING.md. #464

merged 18 commits into from
May 14, 2024

Conversation

VJalili
Copy link
Member

@VJalili VJalili commented Nov 28, 2022

This PR adds the contributing.md that includes guidelines on code contribution and bug reporting.

@VJalili VJalili requested a review from mwalker174 November 28, 2022 19:34
Copy link
Collaborator

@mwalker174 mwalker174 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking the initiative on this! Can you put the text into a Google doc? I think we will need to draft this a bit before merging.

@VJalili
Copy link
Member Author

VJalili commented Jan 4, 2023

That is a good point; I will share the Google doc with you.

Copy link
Collaborator

@mwalker174 mwalker174 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for writing this up. A lot of good info here, particularly for newer devs who may not be familiar with github workflows.

I think we still need a few more sections for some steps that we typically go through before putting up a PR.

For WDL updates,

  1. Updating corresponding template and inputs jsons in /inputs
  2. Rebuilding inputs with build_default_inputs.sh as described in the README
  3. Validating with womtool and miniwdl using the scripts in /scripts/test
  4. Performing a test run on Terra (standalone Cromwell servers are to be deprecated soon)
  5. Information on updating the Terra workspace configurations would also be useful

For backend code (i.e. mostly stuff in /src and /dockerfiles:

  1. Rebuilding and pushing the docker with build_docker.py
  2. Running a test in Terra
  3. Updating dockers.json and dockers_azure.json if necessary, i.e. when it's not an auto-built docker

This is getting to be a long list, so maybe these belong on the website as a part of development documentation, but if we are going to expect users to actually commit changes and post PRs, they will need the rest of this information to get to that point.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved

You may open the code base in any text editor or integrated development environment (IDE).
We recommend using [PyCharm](https://www.jetbrains.com/pycharm/) or
[Visual Studio](https://visualstudio.microsoft.com/vs/community/),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does VS have a WDL plugin? If not we may want to just suggest pycharm.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@mwalker174 mwalker174 merged commit ce9b518 into broadinstitute:main May 14, 2024
1 check passed
@VJalili VJalili deleted the contributing branch May 14, 2024 15:22
@VJalili
Copy link
Member Author

VJalili commented May 14, 2024

Thanks, @mwalker174! Those points are all great; we can extend this contributing file or add a section to the docs to explain these steps in detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants