Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify install.ps1 and move core module files under src/ #376

Merged
merged 7 commits into from
Jan 26, 2017
Merged

Simplify install.ps1 and move core module files under src/ #376

merged 7 commits into from
Jan 26, 2017

Conversation

rkeithhill
Copy link
Collaborator

This moves the core module files under a src dir. BTW can we move install.ps1 to the chocolatey dir? Shipping this file in the module may lead to confusion. Also both it and CheckVersion.ps1 check if git is installed.

dahlbyk
dahlbyk requested changes Jan 26, 2017
View reviewed changes
Copy link
Owner

@dahlbyk dahlbyk left a comment

Choose a reason for hiding this comment

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

BTW can we move install.ps1 to the chocolatey dir? Shipping this file in the module may lead to confusion. Also both it and CheckVersion.ps1 check if git is installed.

Let's leave install.ps1 in the project root, for those who get posh-git via git clone or ZIP download. It is greatly simplified by #358, which I haven't had a chance to test/finish yet, and we shouldn't need to ship it with the Chocolatey package at all.

test/Utils.Tests.ps1
@@ -21,7 +22,7 @@ Describe 'Utils Function Tests' {
Get-FileEncoding $profilePath | Should Be 'utf8'
$content = Get-Content $profilePath
$content.Count | Should Be 2
$modulePath = Resolve-Path $PSScriptRoot\..
# $modulePath = Resolve-Path $PSScriptRoot\..
Copy link
Owner

Choose a reason for hiding this comment

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

Delete?

@rkeithhill rkeithhill changed the title WIP sample of reorg'd dir structure. Reorg dir structure to add src dir. Jan 26, 2017
@rkeithhill
Copy link
Collaborator Author

Will this cause merge headaches for the outstanding PRs?

@dahlbyk dahlbyk force-pushed the rkeithhill/migrate-core-files-to-src-dir branch from a6aafd7 to 92bfdfc Compare January 26, 2017 08:09
@dahlbyk
Copy link
Owner

dahlbyk commented Jan 26, 2017

Will this cause merge headaches for the outstanding PRs?

Kind of. I wasn't comfortable with the overlap between this and #358, so I've overwritten your changes here based on the non-Chocolatey changes that ended up over there. Net result:

  • profile.example.ps1 has been moved back up to the project root, to avoid breaking those who have used install.ps1 or Chocolatey until now.
  • I also added a warning to that file indicating it will be removed in a future version (presumably 1.0).
  • install.ps1 no longer uses profile.example.ps1, instead calling Import-Module directly.
  • Finally, the module files are moved under src/ along with some missing fixes for references to those files.

Note: there are currently a few commits in this branch marked as fixup!. If you see that, it should be a clear indication that the branch is meant to be rebase -i'd before it's merged (autosquash FTW).

@dahlbyk dahlbyk mentioned this pull request Jan 26, 2017
Merged
@dahlbyk dahlbyk added this to the v0.7 milestone Jan 26, 2017
@dahlbyk dahlbyk changed the title Reorg dir structure to add src dir. Simplify install.ps1 and move core module files under src/ Jan 26, 2017
@rkeithhill
Copy link
Collaborator Author

So should I run git rebase -i master on this branch and then git push --force-with-lease before merging it?

@rkeithhill rkeithhill force-pushed the rkeithhill/migrate-core-files-to-src-dir branch from 9524846 to fa7941b Compare January 26, 2017 17:07
@rkeithhill rkeithhill merged commit a77ae39 into master Jan 26, 2017
@rkeithhill rkeithhill deleted the rkeithhill/migrate-core-files-to-src-dir branch January 26, 2017 17:08
@rkeithhill rkeithhill mentioned this pull request Jan 27, 2017
Closed
@dahlbyk
Copy link
Owner

dahlbyk commented Jan 27, 2017

So should I run git rebase -i master on this branch and then git push --force-with-lease before merging it?

Yep, looks good as merged.

@dahlbyk dahlbyk mentioned this pull request Jan 30, 2017
Merged
nweddle added a commit to chef-cft/wombat that referenced this pull request Jan 31, 2017
@nweddle
Fix posh-git PowerShell module loading
1e4d85d 
We simply need to call out that we want to load the post-git PowerShell module.
Caused by breaking change from the posh-git project: dahlbyk/posh-git#376
@nweddle nweddle mentioned this pull request Jan 31, 2017
Merged
@bgshacklett bgshacklett mentioned this pull request Apr 15, 2017
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dahlbyk dahlbyk dahlbyk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.7
Development

Successfully merging this pull request may close these issues.
2 participants
@rkeithhill @dahlbyk