Skip to content

[eight] Backport of my prior updates in Git (Pull #1649)#1655

Merged
sa2ajj merged 6 commits intobuildbot:eightfrom
jaredgrubb:jgrubb-git-inline-1
May 9, 2015
Merged

[eight] Backport of my prior updates in Git (Pull #1649)#1655
sa2ajj merged 6 commits intobuildbot:eightfrom
jaredgrubb:jgrubb-git-inline-1

Conversation

@jaredgrubb
Copy link
Copy Markdown
Member

This backports from the change merged in Pull #1649:

  • move a couple functions to inlineCallbacks
  • remove unused variable
  • also fix a pylint complaint

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This was suggested by pylint, saying the member function had no 'self'.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Generally interfaces don't list self, but that confuses pylint. It doesn't hurt to have self, so this is fine.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need this function at all?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, we do and it was I who wrote it :(

@sa2ajj sa2ajj added this to the eight milestone May 6, 2015
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we use git_path anywhere else?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, it's not. the line was getting a bit long and I wanted to split it... Plus, giving things a name can help readability and make the lines easier to read.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sometimes it helps, and sometimes it does not.

/me is not going to argue here :)

@jaredgrubb
Copy link
Copy Markdown
Member Author

Adjusted the comma as requested.

sa2ajj pushed a commit that referenced this pull request May 9, 2015
[eight] Backport of my prior updates in Git (Pull #1649)
@sa2ajj sa2ajj merged commit 117aea0 into buildbot:eight May 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants