Closed mareuter closed 8 years ago
Hi Michael,
Looks good but there's a large number of trivial whitespace changes that obscure the real one-line change.
Sent from my Android device with K-9 Mail. Please excuse my brevity.
On July 15, 2015 3:55:37 PM PDT, Michael Reuter notifications@github.com wrote:
The dependency is necessary if you install into a fresh conda environment. You can view, comment on, or merge this pull request online at:
https://github.com/mjuric/conda-lsst/pull/28
-- Commit Summary --
- Adding requests as dependency for sims_operations.
-- File Changes --
M bin/conda-lsst (19)
-- Patch Links --
https://github.com/mjuric/conda-lsst/pull/28.patch https://github.com/mjuric/conda-lsst/pull/28.diff
Reply to this email directly or view it on GitHub: https://github.com/mjuric/conda-lsst/pull/28
Sure, I can do that. If you don't know, you can add a ?w=1 to the end of the diff URL to ignore the whitespace: https://github.com/mjuric/conda-lsst/pull/28/files?w=1
The last one is getting fiddly and I'd rather not make too many attempts at it. I've taken care of all the others.
Hi Michael,
I cleaned up, rebased, and merged the PR (see https://github.com/mjuric/conda-lsst/commit/70565dc0967eeb23035637da908577381c87f810). Thanks!
I know it's possible to ask git to hide whitespaces, but they're still there in the commits. While it may seem inconsequential, it's problematic in the long-run as it makes git blame
less useful when debugging and tracing changes back through commit history (yes, there's the -w
for git blame
as well, but not all tools/scripts know about it). It's better to fix it right away.
Also, it's good to rebase and squash all the various 'fixup' commits at the end of the ticket -- again, makes the commit history more useful (git rebase -i master
usually does the trick).
The dependency is necessary if you install into a fresh conda environment.