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

update to plotting_docs #10

Merged
merged 2 commits into from
Nov 28, 2020
Merged

update to plotting_docs #10

merged 2 commits into from
Nov 28, 2020

Conversation

mqharris
Copy link
Owner

Changed some things
I couldnt get save location to work the way I wanted, so I removed that option

Made a PR because I was changing things that I wanted review on.
Also its pretty weird to git push to some one else's branch directly IMO

"""
Function to save plot as jpg, only on MacOS
Hard Coded to save to the current working directory
Copy link
Collaborator

Choose a reason for hiding this comment

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

recommend changing to just state "Saves to current working directory"

@WraySmith
Copy link
Collaborator

I see what you mean about pushing to someone's branch but at some point reviewing every change/accepting back-and-forth becomes bloat. Similar to when editing a document just saying "feel free to make changes required, don't bother doing them in tracked changes as I don't feel the need to review minor stuff". For more significant changes or things that are ambiguous or comments, I would agree but I think minor modifications (spelling changes, minor simple fixes, etc) shouldn't warrant the effort - aka don't make the comment, just make the minor change required.... definitely depends on the process deciding to work on though.... philosophical

@mqharris mqharris merged commit e7268c7 into plotting_docs Nov 28, 2020
@mqharris mqharris deleted the plotting_docs_addendum branch November 28, 2020 22:21
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