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

add watch_topic #31

Closed
wants to merge 5 commits into from
Closed

add watch_topic #31

wants to merge 5 commits into from

Conversation

glanzel
Copy link

@glanzel glanzel commented Apr 13, 2020

Summary of changes

adds api call for: user gets notifications about a topic.

Checklist

  • Changes represent a discrete update
  • Commit messages are meaningful and descriptive
  • Changeset does not include any extraneous changes unrelated to the discrete change

glanzel and others added 2 commits April 14, 2020 00:45
adds: user gets notifications about a topic
Copy link
Collaborator

@bennylope bennylope left a comment

Choose a reason for hiding this comment

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

__init__.py renamed to ahaha__init__.py?

@glanzel
Copy link
Author

glanzel commented May 13, 2020

sorry for the one useless commit. I just didn't excpected the pull request gets resolved right after i tried out some stuff . Now everything is back in place.

@glanzel
Copy link
Author

glanzel commented Jul 9, 2020

I just wanted to clarify that my commit is ready for pulling since 2020/05/14.
I think it is a usefull commit as it adds additional features. I needed this change for my work and would like to see it in your respository. As I am new to use Pull request in github please feel free to give me hints if something is wrong with my request.

@bennylope
Copy link
Collaborator

The feature would be a nice addition, but there's some unnecessary commits and changes here.

I started to clean that up - which is not a big deal - but I'm confused about why the api_user is reset in the watch_topic method.

"""
Suspend a user's account

Args:
userid: the Discourse user ID
duration: the length of time in days for which a user's account
should be suspended
should be suspended (365000 for forever)
Copy link
Collaborator

Choose a reason for hiding this comment

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

?

Copy link
Collaborator

@bennylope bennylope left a comment

Choose a reason for hiding this comment

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

Why is this resetting the api_username?

Returns:

"""
self.api_username = username
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this resetting the api_username? That's the username to connect to the API with, and it's already set when creating the client instance. Changing it here would presumably result in authentication errors trying to make requests to the Discourse API...

@goetzk goetzk added this to the v1.2 milestone Aug 31, 2020
@bennylope
Copy link
Collaborator

Closing this, if requested changes are made will re-open.

@bennylope bennylope closed this Nov 10, 2020
@glanzel
Copy link
Author

glanzel commented Dec 7, 2020 via email

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

Successfully merging this pull request may close these issues.

None yet

3 participants