Skip to content

Conversation

@EricCousineau-TRI
Copy link
Contributor

@EricCousineau-TRI EricCousineau-TRI commented Feb 10, 2021

What does this PR do?

Provides concrete example of running a single file, as is shown in other docs.

Before submitting

  • (N/A) Was this discussed/approved via a GitHub issue? (not for typos and docs)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • (N/A) Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests? (not for typos and docs)
  • Did you verify new and existing tests pass locally with your changes?
  • (N/A) Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:

  • Is this pull request ready for review? (if not, please submit in draft mode)
  • Check that all items from Before submitting are resolved
  • Make sure the title is self-explanatory and the description concisely explains the PR
  • Add labels and milestones (and optionally projects) to the PR so it can be classified
  • Check that target branch and milestone match!

Did you have fun?

Yayyyy! 🎿

@codecov
Copy link

codecov bot commented Feb 10, 2021

Codecov Report

Merging #5910 (a37c60c) into master (b5d29df) will decrease coverage by 42%.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           master   #5910     +/-   ##
========================================
- Coverage      88%     46%    -42%     
========================================
  Files         170     170             
  Lines       11805   11648    -157     
========================================
- Hits        10399    5336   -5063     
- Misses       1406    6312   +4906     

Makefile Outdated

# specific file
# python -m coverage run --source pytorch_lightning -m pytest --flake8 --durations=0 -v -k
# example for specific file/test
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that we do not need it here makefile anymore...
just if it is missing add it to the contribution guide

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! There's already a mention of how to do this in the section for adding a new test, but I've also added a concrete example for an existing test.

@EricCousineau-TRI EricCousineau-TRI changed the title Makefile: Add concrete example of running single-module test Makefile: Remove single-file example; redirect to CONTRIBUTING docs Feb 10, 2021
Base automatically changed from master to release/1.1.x February 11, 2021 14:30
@Borda Borda changed the base branch from release/1.1.x to master February 12, 2021 10:01
@Borda
Copy link
Collaborator

Borda commented Feb 12, 2021

@EricCousineau-TRI thank you for sending your PR, just a minor issue coming from our side... we have swapped branches regarding upcoming feat 1.2, mind rebase on actual master? if needed see How to fix PR with mixed base and target branches?

@EricCousineau-TRI
Copy link
Contributor Author

Sounds good! I'll wait for #5909 to land before rebasing.

@Borda Borda added the waiting on author Waiting on user action, correction, or update label Feb 15, 2021
@Borda
Copy link
Collaborator

Borda commented Feb 15, 2021

Sounds good! I'll wait for #5909 to land before rebasing.

hi @EricCousineau-TRI your mentioned PR is merged, let's get this done too :]

CONTRIBUTING: Add concrete example for running single test
@EricCousineau-TRI EricCousineau-TRI force-pushed the feature-makefile-example-plz branch from f91990c to 729cb99 Compare February 15, 2021 18:17
@EricCousineau-TRI EricCousineau-TRI changed the title Makefile: Remove single-file example; redirect to CONTRIBUTING docs Makefile: Refer to CONTRIBUTING doc, reword test to avoid "example" Feb 15, 2021
@EricCousineau-TRI
Copy link
Contributor Author

Done!

@mergify mergify bot removed the has conflicts label Feb 15, 2021
@Borda Borda added this to the 1.2 milestone Feb 15, 2021
@Borda Borda enabled auto-merge (squash) February 15, 2021 18:38
@carmocca carmocca added ready PRs ready to be merged and removed waiting on author Waiting on user action, correction, or update labels Feb 15, 2021
@Borda Borda merged commit 4f63942 into Lightning-AI:master Feb 15, 2021
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.

5 participants