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

Port to TelepathyGLib #14

Merged
merged 1 commit into from
Jun 13, 2019
Merged

Port to TelepathyGLib #14

merged 1 commit into from
Jun 13, 2019

Conversation

Aniket21mathur
Copy link
Contributor

@Aniket21mathur Aniket21mathur commented May 25, 2019

@quozl @pro-panda @chimosky please review. Thanks!

@quozl
Copy link
Contributor

quozl commented May 27, 2019

Please test before commit, thanks.

@quozl quozl changed the title Port to TelapthyGlib Port to TelapathyGLib May 29, 2019
@quozl quozl changed the title Port to TelapathyGLib Port to TelepathyGLib May 29, 2019
@Aniket21mathur
Copy link
Contributor Author

Tested activity before telepathy port, it seems that the collaboration for activity is already broken. See #15 . Thanks!

@quozl
Copy link
Contributor

quozl commented May 30, 2019

It was your use of get_shared_activity function without calling it that told me it wasn't tested. 😁

@Aniket21mathur
Copy link
Contributor Author

Thanks 😅 ,didn't noticed that. I will wait until #15 is fixed.

@quozl
Copy link
Contributor

quozl commented Jun 13, 2019

Thanks. Reviewed. Deja vu.

  • you've done flake8 changes mixed with other changes; this make reviewing much harder, please try to do these in a different pull request next time,
  • when wrapping argument lists, please do it intelligently, allowing more than one argument per line, as the style you've chosen unnecessarily consumes lines, (limits comprehension of source code when a new developer reads the code in an editor).
  • you've not finished flake8 changes; python2 -m flake8 . still gives three hits,
  • commit message has typo,
  • missing end of line mark on .flake8 file.

Suggestions for workflow;

  • use flake8 before commit,
  • review every change, because somebody else will, e.g. use interactive git add, or a tool that calls it, (my choice is magit in emacs, but you may have other choices),
  • discard changes you don't want to make,
  • take great care with automatic formatters.

@Aniket21mathur
Copy link
Contributor Author

Thanks, will surely take care from next pr, I removed flake8 commits as I will open a separate pr for flake8. 😅 .

@quozl
Copy link
Contributor

quozl commented Jun 13, 2019

Ok, thanks.

@quozl quozl merged commit 9cb9c29 into sugarlabs:master Jun 13, 2019
@Aniket21mathur Aniket21mathur deleted the Telepathy branch June 13, 2019 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants