Skip to content

Conversation

@carmocca
Copy link
Contributor

@carmocca carmocca commented Jan 18, 2021

What does this PR do?

  • Restructure sections, from most general to most specific
  • Add myself to some sections
  • Add @Borda to some sections
  • Add all packages. Assigned members to these new sections.
  • Add @tchaton to API so non-API changes in these files are easier to approve. For example, docstring updates or minor bugs.

cc @Borda

@codecov
Copy link

codecov bot commented Jan 18, 2021

Codecov Report

Merging #5561 (328b920) into master (4dfbbcc) will increase coverage by 3%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #5561    +/-   ##
=======================================
+ Coverage      90%     93%    +3%     
=======================================
  Files         134     134            
  Lines       10055   10055            
=======================================
+ Hits         9040    9388   +348     
+ Misses       1015     667   -348     

@Borda Borda enabled auto-merge (squash) January 18, 2021 21:56
@tchaton
Copy link
Contributor

tchaton commented Jan 19, 2021

Hey @carmocca,

Would you mind adding Justus to /pytorch_lightning/core and both @awaelchli @justusschock for Trainer.

Best regards,
T.C

@carmocca carmocca added the ready PRs ready to be merged label Jan 19, 2021
Copy link
Contributor

@tchaton tchaton left a comment

Choose a reason for hiding this comment

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

LGTM !

@edenlightning
Copy link
Contributor

@carmocca i think this looks good accept for the API files- we still need will to approve them. Is it possible to have both tchaton and williamFalcon approve the API files?

@carmocca
Copy link
Contributor Author

Is it possible to have both tchaton and williamFalcon approve the API files?

The only reason for adding @tchaton to these API files is to avoid blocking small changes to these files (docs updates, formatting changes, minor bug fixes, non-api refactors, typing improvements...)

This requires putting trust on him (great power, great responsability 🕷️). As he would have the power to circumvent @williamFalcon's approval

If you/he are not okay with this. It is better to keep only @williamFalcon as the only CODEOWNER of these files because we wouldn't be gaining anything by requiring both.

@Borda
Copy link
Collaborator

Borda commented Jan 25, 2021

yeas the main reason is just not to be blocked with simple typos... @edenlightning do you have any other suggestion? 🐰

@Borda Borda merged commit 8cb2451 into master Jan 28, 2021
@Borda Borda deleted the update-CODEOWNERS branch January 28, 2021 09:49
tchaton pushed a commit that referenced this pull request Feb 5, 2021
* Update CODEOWNERS

* Update CODEOWNERS

* Apply suggestions from code review

Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>

* Apply suggestions from code review

Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>
Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: edenlightning <66261195+edenlightning@users.noreply.github.com>
Borda added a commit that referenced this pull request Feb 5, 2021
* Update CODEOWNERS

* Update CODEOWNERS

* Apply suggestions from code review

Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>

* Apply suggestions from code review

Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>
Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: edenlightning <66261195+edenlightning@users.noreply.github.com>
Borda added a commit that referenced this pull request Feb 5, 2021
* Update CODEOWNERS

* Update CODEOWNERS

* Apply suggestions from code review

Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>

* Apply suggestions from code review

Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>
Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: edenlightning <66261195+edenlightning@users.noreply.github.com>
@carmocca carmocca self-assigned this Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready PRs ready to be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants