Skip to content

development process updates#82

Merged
edublancas merged 9 commits into
mainfrom
changes
Jul 11, 2023
Merged

development process updates#82
edublancas merged 9 commits into
mainfrom
changes

Conversation

@edublancas
Copy link
Copy Markdown
Contributor

@edublancas edublancas commented Jul 10, 2023

updated the contribution guidelines based on feedback I've heard from the team


📚 Documentation preview 📚: https://ploomber-contributing--82.org.readthedocs.build/en/82/

Comment thread contributing/process-outline.md Outdated
For more information, see [](submitting-pr.md).
```

- Once you've completed the work, submit a draft pull request to the repository.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can mention here that draft is when it's still work in progress. Need to mark ready for review when work is completed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Few typos here:
opem, clicck

Comment thread contributing/review.md Outdated
Comment thread contributing/review.md
Comment thread contributing/review.md
Comment thread contributing/submitting-pr.md Outdated
Comment thread contributing/submitting-pr.md
@neelasha23
Copy link
Copy Markdown
Contributor

Added some comments. Do you think it would help if there's a simple flowchart diagram to help understand which tutorial to refer in which scenario? common scenarios maybe reviewing code (then check out Reviewing PRs, Maintainer's Guide..)
@edublancas

Copy link
Copy Markdown
Contributor

@yafimvo yafimvo left a comment

Choose a reason for hiding this comment

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

minor typos

Comment thread contributing/process-outline.md Outdated
you can choose one that you're interested in working on.
## 1. Getting issue assigned

The project's maintainers may assign you an issue, or you can choose one that you're
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

or you can -> or you can (double space)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Comment thread contributing/submitting-pr.md Outdated

Check the logs and fix them (see the [troubleshooting](troubleshooting.md) guide
for tips). If you're unable to fix the issues after spending some time on them. You
can message us on Slack so we can help you.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you're unable to fix the issues after spending some time on them. You
can message us on Slack so we can help you.

I think it should be one sentence

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Comment thread contributing/submitting-pr.md Outdated

Ensure there are no [merge conflicts](#fixing-merge-conflicts).

If all the test pass and there are no merge conflicts, you can [request a review](#requesting-a-review).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

conflicts, you can -> conflicts, you can (double space)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

@edublancas edublancas requested review from neelasha23 and yafimvo July 11, 2023 14:52
@edublancas
Copy link
Copy Markdown
Contributor Author

thanks for the feedback, addressed all the comments!

Do you think it would help if there's a simple flowchart diagram to help understand which tutorial

@neelasha23: we have some quick access links in the home page. do you think this can help? Or are you thinking that we can include an actual diagram (perhaps in the outline section?). adding the diagram would be simple with mermaid: https://mermaid.js.org/intro/

@neelasha23
Copy link
Copy Markdown
Contributor

Or are you thinking that we can include an actual diagram (perhaps in the outline section?). adding the diagram would be simple with mermaid: https://mermaid.js.org/intro/

Yes if it's not too much of a task maybe a visual chart would be easier and quicker to understand.

Also, I can't see resolve conversation buttons anywhere
Screenshot 2023-07-11 at 10 29 51 PM

@edublancas

@edublancas
Copy link
Copy Markdown
Contributor Author

@neelasha23 thanks for the heads up on the "resolve conversation" button. I fixed github permissions (I'll add a little note so people can notify us if they're lacking permissions). please resolve the conversations if you're ok with the changes. I'll open a new issue with the flowchart

neelasha23
neelasha23 previously approved these changes Jul 11, 2023
Copy link
Copy Markdown
Contributor

@neelasha23 neelasha23 left a comment

Choose a reason for hiding this comment

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

Added a comment on typos. Looks good otherwise!

@edublancas edublancas merged commit bcfca47 into main Jul 11, 2023
@edublancas edublancas deleted the changes branch July 11, 2023 17:49
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.

3 participants