Skip to content

Create separate Top-level directory per platform - #1477

Merged
sergei (vors) merged 4 commits into
masterfrom
vors/config
Jul 23, 2016
Merged

Create separate Top-level directory per platform#1477
sergei (vors) merged 4 commits into
masterfrom
vors/config

Conversation

@vors

@vors sergei (vors) commented Jul 22, 2016

Copy link
Copy Markdown
Collaborator
  • Replace src\powershell top level project by two
    new root-level projects:
    • src\powershell-unix
    • src\powershell-windows
  • Break src\Modules into more granular structure:
    • Shared
    • Windows+Unix-Core
    • Windows-Core
    • Windows-Core+Full
    • Windows-Full
    • To be created: Unix-Core. We will do it as a separate PR
  • Fix Platform-specific set of Modules (and assemblies) #1122 : Platform-specific set of Modules (and assemblies)

@andyleejordan Andy Jordan (andyleejordan) mentioned this pull request Jul 22, 2016
Closed
6 of 11 tasks
andyleejordan
andyleejordan reviewed Jul 22, 2016
View reviewed changes
Comment thread src/Modules/README.md Outdated

@andyleejordan Andy Jordan (andyleejordan) Jul 22, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm reading this as Windows Plus Unix Minus Core, which I don't think is what it means.

@vors sergei (vors) Jul 22, 2016

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We are mirroring nuget framework notation here

@vors sergei (vors) mentioned this pull request Jul 22, 2016
Merged
2 of 4 tasks
daxian-dbw
daxian-dbw reviewed Jul 22, 2016
View reviewed changes
Comment thread build.psm1 Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Jul 22, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are we building the CoreCLR version native powershell.exe and pwrshplugin.dll for powershell-windows?

@vors sergei (vors) Jul 22, 2016

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We don't build native powershell.exe, we do build pwrshplugin.
I asked Mike add it to the package in #1490 (comment)

@daxian-dbw

Dongbo Wang (daxian-dbw) commented Jul 22, 2016

Copy link
Copy Markdown
Member

Two minor comments:

  1. powershell-windows seems not clear enough to indicate that it's CoreCLR on windows
  2. the matrix of module folder names, would the following be better?
    Shared-All
    Shared-Core
    Shared-Win
    Win-Core
    Win-Full
    Unix-Core

@daxian-dbw

Dongbo Wang (daxian-dbw) commented Jul 22, 2016

Copy link
Copy Markdown
Member

LGTM except for two minor comments.

@vors

sergei (vors) commented Jul 22, 2016

Copy link
Copy Markdown
Collaborator Author

I agree with the first one partially, but keeping carry around "core" is verbose.
We have a clear explanation in docs\building\internals.md and src\powershell\README.md and without these verbose explanations I don't think anybody can understand what's going on with the build, just by looking at the folder names. :(

For the second, that's really a matter of preference "how to represent matrix in a flat way". I don't like Shared-Core and Shared-Win together, because they use different entities at the same places and it's confusing.
I realize that current way is confusing too, but for long term, we have a better plan (with $IsLinux, $IsCoreCLR in the .psd1 files).

So I would probably keep these two things as is. Thank you for the thoughtful feedback!

Sergei Vorobev added 4 commits July 22, 2016 16:11
Sergei Vorobev
Create separate Top-level directory per platform
875402a 
- Replace src\powershell top level project by two
  new root-level projects:
    - src\powershell-unix
    - src\powershell-windows
- Break src\Modules into more granular structure:
  - Shared
  - Windows+Unix-Core
  - Windows-Core
  - Windows-Core+Full
  - Windows-Full
  - To be created: Unix-Core. We will do it as a separate PR
- Fix #1122 : Platform-specific set of Modules (and assemblies)
- Fix Start-TypeGen to work with new top level folders layout
Sergei Vorobev
Copy pwrshplugin into the Full CLR version artifacts
3674bac
Sergei Vorobev
Update .gitignore to ignore dlls and pdbs
c4deb5d
Sergei Vorobev
Update building/internals.md docs
7f274a3
@andyleejordan Andy Jordan (andyleejordan) mentioned this pull request Jul 22, 2016
Closed
6 tasks done
@vors
sergei (vors) merged commit 28a7ca2 into master Jul 23, 2016
@vors
sergei (vors) deleted the vors/config branch July 23, 2016 00:11
This was referenced Jul 28, 2016
Closed
Closed
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@vors
Merge pull request PowerShell#1477 from PowerShell/vors/config
be121c1 
Create separate Top-level directory per platform
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vors @daxian-dbw @andyleejordan