dsccommunity / StorageDsc

DSC resource module is used to manage storage on Windows Servers.
https://dsccommunity.org
MIT License
69 stars 51 forks source link

Update CHANGELOG.md #216

Closed fullenw1 closed 4 years ago

fullenw1 commented 4 years ago

Pull Request (PR) description

Added information about issue 214 for the Disk resource.

This Pull Request (PR) fixes the following issues

- Fixes #214 

Task list


This change is Reviewable

codecov-io commented 4 years ago

Codecov Report

Merging #216 into dev will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@        Coverage Diff         @@
##           dev   #216   +/-   ##
==================================
  Coverage   95%    95%           
==================================
  Files        7      7           
  Lines      985    985           
==================================
  Hits       943    943           
  Misses      42     42
PlagueHO commented 4 years ago

Hi @fullenw1 - you can just push this change to your patch-2 branch and it will automatically show up in the existing PR :grin: that saves you needing two PRs.

Great job on this BTW! Thank you

fullenw1 commented 4 years ago

you can just push this change to your patch-2 branch and it will automatically show up in the existing PR that saves you needing two PRs.

I'm afraid I don't understand... Where should I do that?

I tried to edit the first line: fullenw1 wants to merge 1 commit into PowerShell:dev from fullenw1:patch-3 But when I try to change PowerShell:dev to fullenw1:patch-2 it does not appear in the list. Or maybe am I trying to change the wrong thing... :-/

PlagueHO commented 4 years ago

Hi @fullenw1 - what you should do is:

  1. Change to your patch-2 branch on your local machine: git checkout patch-2
  2. Merge your changes from patch-3 into patch-2 by: git merge patch-3
  3. Push patch 2 with: git push

This should then update the patch-2 PR. Forgive any mistakes (I'm typing this off the top of my head and I'd usually have tab complete to help me remember the appropriate Git commands :grin:)

fullenw1 commented 4 years ago
  1. Change to your patch-2 branch on your local machine: git checkout patch-2
  2. Merge your changes from patch-3 into patch-2 by: git merge patch-3
  3. Push patch 2 with: git push

git : merge: patch-3 - not something we can merge

I'm sorry, I'm a bit new to git and github... I think all my modifications have created unnecessary branches :-(

By the way, version 4.9.0.0 has already been released. I guess the easiest way is to create a new single branch and a new pull request... What do you think?

PlagueHO commented 4 years ago

Hi @fullenw1 - sorry about the delay. I'm thinking the best thing to do might be to just create a new branch with the changes from the latest Dev branch and then submit a new PR and we'll drop the old ones.

E.g. (I'm assuming your remote for PowerShell/StorageDsc repos is called upstream and your fullenw1/StorageDsc remote is called origin)

git checkout dev
git pull upstream dev
git checkout -B "Patch-5"
.... make all changes (just combine all the changes into a single PR as they are all small- don't forget CHANGELOG.MD).....
git push --set-upstream origin Patch-5

Submit new PR from Patch-5 and we'll close the other ones.

fullenw1 commented 4 years ago

Hi @PlagueHO

I think we are near the end now... :D Here is the pull request: #218

PlagueHO commented 4 years ago

Sorry about the delay @fullenw1 - I can't merge till the tests pass and they're failing due to a recent change we made elsewhere. So I'm just going to fix that and then we should be able to merge.

Although there is one issue: The CHANGELOG.MD entry you added is too long and causes this test failure: https://ci.appveyor.com/project/PowerShell/storagedsc/builds/28514697?fullLog=true#L630

Should be super simple to fix.

fullenw1 commented 4 years ago

Although there is one issue: The CHANGELOG.MD entry you added is too long and causes this test failure: https://ci.appveyor.com/project/PowerShell/storagedsc/builds/28514697?fullLog=true#L630

Ok @PlagueHO. Do you mean the line is more than 80 characters long? Also which branch am I supposed to fix?

PlagueHO commented 4 years ago

Closing because superseded by #218