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

Added PyUnusedCodeBear to .coafile #582

Merged
merged 1 commit into from
Jul 18, 2018
Merged

Conversation

Man-Jain
Copy link
Member

@Man-Jain Man-Jain commented Jul 13, 2018

.coafile : Added PyUnusedCodeBear to [all.python] section

Added the PyUnusedCodeBear to .coafile in [all.python] section and then ran coala command which removes the unused imports from project files.

Fixes :- #531

.coafile Outdated
@@ -27,6 +27,10 @@ bears = LineLengthBear
[all.links]
bears = InvalidLinkBear

[all.unusedcode]
Copy link
Member

Choose a reason for hiding this comment

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

No need to make a separate section. You can include it under [all.python].
Also, read the commit guidelines and modify your commit according to it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay I will do the changes.

@Man-Jain
Copy link
Member Author

@nvzard Sorry, But I am not able to understand why the build is failing on semaphoreci. It would be great if you could help me.

@nvzard
Copy link
Member

nvzard commented Jul 14, 2018

@Man-Jain sorry Internet was down whole day due to govt orders.
You need to squash your commits and go through the commit guideline as I said before. Make sure you push your code from a separate branch and not the master branch.

Push again, most probably a restart will fix the semaphore build failure.

Also, you need to rebase.

@Man-Jain
Copy link
Member Author

@nvzard Surprisingly the internet was down in our city also due to Govt. Orders. I have done the other changes but in order to push from a a differnt branch I will have to create a new pull request. So should I close this Pull Request and open a new one?

@nvzard
Copy link
Member

nvzard commented Jul 16, 2018

Don't close the PR. Your master is different from corobo's master, just fix that first and amend your changes. Keep it in mind and don't push from master next time.

@Man-Jain
Copy link
Member Author

@nvzard The semaphoreci Docker is still failing.

@nvzard
Copy link
Member

nvzard commented Jul 16, 2018

@meetmangukiya I can't figure out why semaphore.docker is failing, can you please take a look.

@Man-Jain as mentioned in the commit guidelines, short log is written in imperative present tense (i.e. Add something, not Adding something or Added something).
Also, use closes instead of fixes and read the commit guidelines for the reason.

@Man-Jain
Copy link
Member Author

Man-Jain commented Jul 16, 2018

@nvzard I am very sorry for that mistake. I missed that part. I will surely take care of this next time.

for start, end in q_graph.edges():
found_common = True
Copy link
Member

Choose a reason for hiding this comment

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

@Man-Jain don't make changes to answers/* and push your commit again. I am not sure, but this might work.

Copy link
Member

Choose a reason for hiding this comment

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

^Ignore

@nvzard
Copy link
Member

nvzard commented Jul 17, 2018

@Man-Jain Semaphore is failing because you pushed your commit from the master branch. Create a new branch and then push your commit.
You can go through

if [[ $BRANCH_NAME != "master" ]]
and read the CI logs to find the cause.

@jayvdb
Copy link
Member

jayvdb commented Jul 18, 2018

@Man-Jain , dont create a new PR. I will try to fix this one for you.

@@ -110,7 +109,7 @@ def ls(self, msg, match):
Example: `ls bears python python3`
"""
langs = list(map(lambda x: x.lower(), match.group(1).split()))
all_langs = Coatils.all_langs()
Coatils.all_langs()
Copy link
Member

Choose a reason for hiding this comment

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

the returned value wasnt being used, and invoking Coatils.all_langs doesnt have side-effects .
You can remove the line.

(whereas the self.pop_message() in coatils_test.py does have side effects and can not be removed.)

@Man-Jain
Copy link
Member Author

Man-Jain commented Jul 18, 2018

@nvzard Okay, I see I should have carefully made the pull request.

@Man-Jain
Copy link
Member Author

@jayvdb Okay I will make the changes that you have suggested.

@jayvdb
Copy link
Member

jayvdb commented Jul 18, 2018

please do it really quickly, as all other PRs need to wait for your one to be merged

Added the PyUnusedCodeBear to the .coafile and ran coala command.
This bear will remove the unused code from the project files. The bear is added to the [all.python] section.

Fixes coala#531
@jayvdb
Copy link
Member

jayvdb commented Jul 18, 2018

ack 18a5b33

@jayvdb
Copy link
Member

jayvdb commented Jul 18, 2018

@gitmate-bot ff

@gitmate-bot
Copy link

Hey! I'm GitMate.io! This pull request is being fastforwarded automatically. Please DO NOT push while fastforward is in progress or your changes would be lost permanently ⚠️

@gitmate-bot gitmate-bot merged commit 18a5b33 into coala:master Jul 18, 2018
@gitmate-bot
Copy link

Automated fastforward with GitMate.io was successful! 🎉

@nvzard
Copy link
Member

nvzard commented Jul 19, 2018

Yikes! We missed the 72 char commit body limit with this one 😥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants