-
-
Notifications
You must be signed in to change notification settings - Fork 79
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
Fix: Docstrings converted to single-line #427
Conversation
- Docstrings fits in a single line according to PEP8. - Reference https://deepsource.io/gh/scanapi/scanapi/issue/FLK-D200/occurrences Fixes #411
@marcorichetta your pull request is missing a changelog! |
Codecov Report
@@ Coverage Diff @@
## main #427 +/- ##
=======================================
Coverage 95.21% 95.21%
=======================================
Files 22 22
Lines 710 710
=======================================
Hits 676 676
Misses 34 34
Continue to review full report at Codecov.
|
@marcorichetta Everything else LGTM 👍🏾 can you just squash the commit into one ? |
@Pradhvan I squashed commits on merge via github Squash and merge but never made it on the same branch. After reading from Gitlab docs, I was able to squash them but I'm not sure of the final step:
Is it correct to go with |
@marcorichetta yes, you would need to force push after squashing your commit into one. The reason we do this is so that the master branch commits have a cleaner commit history as your change can be easily tracked with one commit rather than having two. Let me know if you need any help with anything. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🌟
Noooooo, I've merged it, sorry! My bad 😢 |
@Pradhvan Thanks for the explanation! I thnk @camilamaia merged the two commits with the Squash and merge option because on main there is only one commit => ccfb432 That said, is there an advantage on squashing locally vs squash on merge? I think the latter is easier both for contributor and for mantainer. |
@marcorichetta yes the latter is an easier option but usually gives out a commit history that is definitely not clean. For example, this is what the master branch's commit message looks like when it was squashed and merged it.
On the other hand, if we had squashed and merged it into a single commit. We would have the liberty to make this message a bit cleaner and more understandable for others who were not involved with the PR. Something like this
The latter commit message helps in keeping a clean commit history so others know what has been merged with master just by looking at the last commit. |
Description
These docstrings fits in a single line according to PEP8.
Reference: https://deepsource.io/gh/scanapi/scanapi/issue/FLK-D200/occurrences
Motivation behind this PR?
Solve DeepSource issues.
What type of change is this?
Fix
Checklist
Issue
Closes #411