Skip to content

feature(watching): Add support for ignoring or subscribing to repos#360

Open
mtscout6 wants to merge 1 commit intogithub-tools:masterfrom
mtscout6:watchers
Open

feature(watching): Add support for ignoring or subscribing to repos#360
mtscout6 wants to merge 1 commit intogithub-tools:masterfrom
mtscout6:watchers

Conversation

@mtscout6
Copy link
Copy Markdown
Member

@mtscout6 mtscout6 commented Jul 3, 2016

@mtscout6
Copy link
Copy Markdown
Member Author

ping

Comment thread lib/Watching.js
* Watching a Repository registers the user to receive notifications on new discussions, as well as events in the user's
* activity feed.
*/
export default class Watching extends Requestable {
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.

At the moment we use module.exports for exporting classes. I'm not sure about the reason as this was made by @clayreimann but I think it's because we aren't using the babel plugin to revert the export of defaults to the behavior of Babel 5.

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.

I didn't know there was such a plugin. I forget the exact reason that we're using module.exports but it boiled down to nothing works if you use the export default syntax.

If there's a plugin that fixes things while allowing us to use the new syntax then I'm all for it.

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.

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.

I've used that plugin before as well. I'll get it wired up.

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.

I suggest to do it in a separate PR.

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.

The commit is already in, it was a two line change and doesn't require me to change this. I can do that if you like though.

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.

If I do a separate PR for this, then I'll tackle the babel-runtime issue as well. I'll crank that out real quick.

Copy link
Copy Markdown
Member Author

@mtscout6 mtscout6 Jul 14, 2016

Choose a reason for hiding this comment

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

Alright it is in PR #366 We should hold off on this PR until that one is done because of this line.

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.

3 participants