DrylandEcology / rSFSW2

rSFSW2: A R package to create soil water balance simulation experiment
GNU General Public License v3.0
8 stars 7 forks source link

Revert "Merge SSURGO extract functionality into master" #314

Closed CaitlinA closed 6 years ago

CaitlinA commented 6 years ago

Reverts DrylandEcology/rSFSW2#304

codecov[bot] commented 6 years ago

Codecov Report

Merging #314 into master will decrease coverage by 1.48%. The diff coverage is 10.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #314      +/-   ##
==========================================
- Coverage    48.4%   46.91%   -1.49%     
==========================================
  Files          39       39              
  Lines       16833    14790    -2043     
==========================================
- Hits         8148     6939    -1209     
+ Misses       8685     7851     -834
Impacted Files Coverage Δ
R/ExtractData_Soils.R 0% <ø> (ø) :arrow_up:
R/Simulation_Project.R 72.77% <0%> (-3.4%) :arrow_down:
R/Soils_Functions.R 54.7% <100%> (-2.72%) :arrow_down:
R/PriorCalculations.R 48.61% <5.88%> (+5.31%) :arrow_up:
R/Indices.R 94.28% <0%> (-5.72%) :arrow_down:
R/IO_datafiles.R 82.48% <0%> (-5.33%) :arrow_down:
R/Timing_Calls.R 95% <0%> (-5%) :arrow_down:
R/Status_Trackers.R 95.23% <0%> (-4.77%) :arrow_down:
R/Testproject_Functions.R 53.39% <0%> (-4.72%) :arrow_down:
R/Pedotransfer_Functions.R 90.32% <0%> (-4.55%) :arrow_down:
... and 27 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f23e87e...65ece19. Read the comment docs.

dschlaep commented 6 years ago

@CaitlinA I re-started the travis-ci that was failing. It was failing because it was timing out. It took so long because it was the first build on this branch/PR, i.e., packages were not cached and had to be re-built which takes too long... So this should be no good to merge. Thanks!

dschlaep commented 6 years ago

@CaitlinA I thought you would clean this up? I merge now.

CaitlinA commented 6 years ago

I reverted the commit, so that I could clean it up on its original separate branch. I have been on vacation and just super busy since I've gotten back, so I haven't done it yet. I repeat - this branch is messy and shouldn't be merged ...

On Wed, Jun 6, 2018 at 11:12 AM, daniel notifications@github.com wrote:

Merged #314 https://github.com/DrylandEcology/rSFSW2/pull/314.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/DrylandEcology/rSFSW2/pull/314#event-1666801216, or mute the thread https://github.com/notifications/unsubscribe-auth/AM0e_9coaIEsPe5x_AzKVerY5gIQBa-Pks5t6BuhgaJpZM4UKzfj .

-- Caitlin Andrews Ecologist Southwest Biological Science Center U.S. Geological Survey Mobile: 802-922-3494 Office: 928-556-7036

dschlaep commented 6 years ago

This PR was to clean up the messed up master. And we need a clean master so that we can base our development work on something stable.

Feature branches can be messy. Cleaning up can be done on the old new_SSURGO or someplace else. But it is not ok to keep master unusable.

CaitlinA commented 6 years ago

I thought that by reverting a pull request, I was taking the commits that were unstable off of master and putting them back on the feature branch.

On Wed, Jun 6, 2018 at 11:24 AM, daniel notifications@github.com wrote:

This PR was to clean up the messed up master. And we need a clean master so that we can base our development work on something stable.

Feature branches can be messy. Cleaning up can be done on the old new_SSURGO or someplace else. But it is not ok to keep master unusable.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/DrylandEcology/rSFSW2/pull/314#issuecomment-395167234, or mute the thread https://github.com/notifications/unsubscribe-auth/AM0e_-jfGQCJWcdBGa7ohJy2lqOu_Q8dks5t6B5tgaJpZM4UKzfj .

-- Caitlin Andrews Ecologist Southwest Biological Science Center U.S. Geological Survey Mobile: 802-922-3494 Office: 928-556-7036

dschlaep commented 6 years ago

Note the difference between git reset and git revert and whether or not commits have been pushed to remote. Plus, nothing gets changed on master except by being merged via an accepted pull request.

So, yes, reverting the pull request was the first step; this created a new pull request (this one #314); this PR had to first pass our tests (which it didn't at first, see my comment above); once passed, it needed to be manually merged into master. Only then are the commits from the new_SSURGO reverted on master, i.e., each change was reverted by a new change (they didn't disappear).

CaitlinA commented 6 years ago

Copy. So where do we go from here? I still need to make changes to the SSURGO code.

On Wed, Jun 6, 2018 at 11:36 AM, daniel notifications@github.com wrote:

Note the difference between git reset and git revert and whether or not commits have been pushed to remote. Plus, nothing gets changed on master except by being merged via an accepted pull request.

So, yes, reverting the pull request was the first step; this created a new pull request (this one #314 https://github.com/DrylandEcology/rSFSW2/pull/314); this PR had to first pass our tests (which it didn't at first, see my comment above); once passed, it needed to be manually merged into master. Only then are the commits from the new_SSURGO reverted on master, i.e., each change was reverted by a new change (they didn't disappear).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/DrylandEcology/rSFSW2/pull/314#issuecomment-395170839, or mute the thread https://github.com/notifications/unsubscribe-auth/AM0e_4z_aLUrAAxor3p0WRT4Wu6heFZ4ks5t6CFLgaJpZM4UKzfj .

-- Caitlin Andrews Ecologist Southwest Biological Science Center U.S. Geological Survey Mobile: 802-922-3494 Office: 928-556-7036

dschlaep commented 6 years ago

Resurrect new_SSURGO?