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.

Sign up for GitHub

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

Jump to bottom

Clean up code formatting #65

Merged
merged 1 commit into from Jun 15, 2021
Merged

Clean up code formatting #65

merged 1 commit into from Jun 15, 2021

Conversation

jackton1
Copy link
Contributor

@jackton1 jackton1 commented Jun 15, 2021 •
edited

Purpose

Improve readability of README.

@Deconstrained
Copy link
Collaborator

That's a massive improvement! Thank you!

@Deconstrained Deconstrained self-requested a review June 15, 2021 23:38
@Deconstrained Deconstrained merged commit 1517f83 into PagerDuty:main Jun 15, 2021
@Deconstrained
Copy link
Collaborator

@jackton1 speaking of readability, what's your opinion on it? Is it too verbose? Biggest mistake I think I've made on this project is doing all the documentation myself as I see appropriate and never really asking for feedback.

@jackton1
Copy link
Contributor Author

jackton1 commented Jun 16, 2021 •
edited

@jackton1 speaking of readability, what's your opinion on it? Is it too verbose? Biggest mistake I think I've made on this project is doing all the documentation myself as I see appropriate and never really asking for feedback.

I think it’s okay I know I’ve been experiencing ConnectionError using requests lately mostly with celery.

  • Some improvements would be to advocate for using the Session as a context manager
  • Also unclear whether PDSession should be used publicly. Using __all__ would help for a case of star imports.
  • The links also don’t work at the moment

@jackton1 jackton1 deleted the patch-1 branch June 16, 2021 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Deconstrained Deconstrained Deconstrained approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jackton1 @Deconstrained