Skip to content
This repository has been archived by the owner on Feb 24, 2021. It is now read-only.

DscResources: Added submodule NetworkingDsc, and xDisk #382

Merged
merged 2 commits into from May 24, 2018

Conversation

johlju
Copy link
Contributor

@johlju johlju commented May 18, 2018

  • Changes to DscResources
    • Added deprecated resource module xDisk for reference.
    • Added renamed resource module NetworkingDsc, and removing old xNetworking.
    • Renamed xNetworking to NetworkingDsc in all related documents.
    • Minor updates to integration tests template.
    • Switched examples in TestGuidelines because the previous resource
      tests had gotten too complex for an easy example.

This change is Reviewable

@johlju johlju changed the title Update repo DscResources: Added submodule NetworkingDsc and xDisk (as deprecated) May 18, 2018
@johlju johlju changed the title DscResources: Added submodule NetworkingDsc and xDisk (as deprecated) DscResources: Added submodule NetworkingDsc, and xDisk (as deprecated) May 18, 2018
@johlju johlju changed the title DscResources: Added submodule NetworkingDsc, and xDisk (as deprecated) DscResources: Added submodule NetworkingDsc, and xDisk May 18, 2018
@johlju
Copy link
Contributor Author

johlju commented May 18, 2018

@PlagueHO Do you mind reviewing this one too when you have a chance? VS Code trimmed a lot of white spaces, so it's look bigger than it is.

@johlju johlju added the needs review The pull request needs a code review. label May 18, 2018
@PlagueHO
Copy link
Contributor

Sweet as @johlju . I'll get on to this and the other ones tomorrow!

@PlagueHO
Copy link
Contributor

Everything looks good, but just a question about whether or not we should still include deprecated resources as submodules?


Reviewed 10 of 10 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


.gitmodules, line 196 at r1 (raw file):

	path = DscResources/ActiveDirectoryCSDsc
	url = https://github.com/PowerShell/ActiveDirectoryCSDsc
[submodule "xDscResources/xDisk"]

Should this resource still be linked as a submodule if it is deprecated? Same goes for xTimezone.


Comments from Reviewable

@johlju
Copy link
Contributor Author

johlju commented May 19, 2018

Review status: all files reviewed at latest revision, 1 unresolved discussion.


.gitmodules, line 196 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Should this resource still be linked as a submodule if it is deprecated? Same goes for xTimezone.

I understand your point. This repo was linked to the Waffle board. I thought is since they is a part of DSC Resource Kit we keep the linked here too so they don't float around without an "owner". I personally rather see these deprecated repos removed entirely later on, and then we can remove and references to them.
We are also discussing the use of submodules in it's entirely, haven't had time posting an issue about that yet (I will make sure to do that next week). I suggest keeping these as submodules for the time being.


Comments from Reviewable

- Added deprecated resource module xDisk for reference.
- Renamed xNetworking to NetworkingDsc
- Minor updates to integration tests template.
- Switched examples in TestGuidelines because the previous resource
  tests had gotten too complex for an easy example.
@johlju
Copy link
Contributor Author

johlju commented May 23, 2018

@PlagueHO Do you have time to sign-off on this one unless you have another argument for us leaving out xDisk? 🙂

@PlagueHO
Copy link
Contributor

Doh! Sorry I missed this @johlju - I'll sign it off first thing tomorrow (only on my phone atm)

@PlagueHO
Copy link
Contributor

:lgtm:


Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@johlju johlju merged commit e2767f6 into PowerShell:master May 24, 2018
@johlju johlju removed the needs review The pull request needs a code review. label May 24, 2018
@johlju johlju deleted the update-repo branch May 24, 2018 08:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants