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

PR template added and gitignore improved #867

Merged
merged 7 commits into from
Oct 11, 2023

Conversation

smttsp
Copy link
Contributor

@smttsp smttsp commented Oct 6, 2023

Summary

What is the reason for this Pull-Request? If adding new functionality, please comment:
a code snippet showcasing the end-to-end usage of your new methods, and the exact output
from this code (print out any returned variables).

This PR adds a PR template to the cleanlab repo (as seen in this PR) as well as adds some extensions/folders to .gitignore to make it more comprehensive.

Impact

What is the scope of your changes and who/where do you think they will affect?

  • improved gitignore
  • new PR template so that PR description structures would be consistent

Testing

What testing have you done and verified? Please link to the one unit test
that is most end-to-end check of your new functionality. It's ok if your
unit tests are not yet comprehensive when you first open the PR,
we can revisit them later!

  • N/A

Ticket(s) or Conversations

What Git or Slack items (Issues, threads, etc) that are specifically related to
this work? Please link them here.

References

Include any extra information that may be helpful to reviewers of your
Pull-Request here (e.g. relevant URLs or Wiki pages that could help give
background information). If relevant, please include additional code snippets
(and their outputs/return values) showing alternative ways to use your newly
added methods.

@CLAassistant
Copy link

CLAassistant commented Oct 6, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor Author

@smttsp smttsp left a comment

Choose a reason for hiding this comment

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

ready for review

@smttsp smttsp marked this pull request as ready for review October 6, 2023 19:48
@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (bd32f11) 96.70% compared to head (08058f7) 96.70%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #867   +/-   ##
=======================================
  Coverage   96.70%   96.70%           
=======================================
  Files          65       65           
  Lines        5091     5091           
  Branches      875      875           
=======================================
  Hits         4923     4923           
  Misses         86       86           
  Partials       82       82           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

.gitignore Outdated Show resolved Hide resolved
Copy link
Member

@jwmueller jwmueller left a comment

Choose a reason for hiding this comment

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

PR template LGTM.

@elisno will let you make suggestions for the .gitignore based on your preferences and merge this.

@jwmueller jwmueller requested a review from elisno October 7, 2023 11:18
Co-authored-by: Jonas Mueller <1390638+jwmueller@users.noreply.github.com>
@tataganesh
Copy link
Contributor

tataganesh commented Oct 9, 2023

This is a great template! As part of the summary/impact, could we also suggest adding screenshots when there is a change in documentation/rendered notebooks? Example - #751.

@smttsp
Copy link
Contributor Author

smttsp commented Oct 11, 2023

@elisno, this is pretty simple PR. It may be nice to merge this soon, so others would be able to start using the template and gitignore soon

Copy link
Member

@elisno elisno 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 the PR, @smttsp!

  • The .gitignore looks pretty good to me.
  • I made a few suggestions on what to change about the PR template.

.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Show resolved Hide resolved
smttsp and others added 2 commits October 10, 2023 23:40
Co-authored-by: Elías Snorrason <eliassno@gmail.com>
Co-authored-by: Elías Snorrason <eliassno@gmail.com>
Copy link
Contributor Author

@smttsp smttsp left a comment

Choose a reason for hiding this comment

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

I will fix spacings and update the clean vision pr template as well once we finalize this

.github/pull_request_template.md Outdated Show resolved Hide resolved
- Enhance instructions for PR purpose and usage examples.
- Add clear, user-friendly placeholders for contributors.
- Ensure code snippets are executable and demonstrative.
- Fix formatting of remaining sections
@smttsp
Copy link
Contributor Author

smttsp commented Oct 11, 2023

@elisno, I am fine with the changes. I think the next step will be that you will take over the branch and merge it, right?

Copy link
Member

@elisno elisno left a comment

Choose a reason for hiding this comment

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

LGTM!

Thank you for your contribution @smttsp!

@elisno elisno merged commit 8b8bf78 into cleanlab:master Oct 11, 2023
@smttsp smttsp deleted the pr_template_gitignore branch October 11, 2023 17:56
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.

None yet

5 participants