Skip to content

Couple of things - #6

Open
ChrisPortman wants to merge 5 commits into
jhoblitt:masterfrom
ChrisPortman:shell
Open

Couple of things#6
ChrisPortman wants to merge 5 commits into
jhoblitt:masterfrom
ChrisPortman:shell

Conversation

@ChrisPortman

@ChrisPortman ChrisPortman commented Jul 23, 2015

Copy link
Copy Markdown

Hi Joshua,

Thanks for the module, makes perlbrew pretty straight forward!

Just a couple of things:

  • I had an issue where the installer set the init file up with setenv commands instead of exports for some reason. Apparently it uses a $SHELL var to work out what shell is in use and then sets accordingly. One of the commits forces bash and the install sorted itself out.
  • I created a new define for switch to separate it from the perl install process and to allow users to manage the switch separately in their profiles. Should also help to support multiple perls per root.
  • Added a switch option to the perl define, to allow the disabling of the switch as part of the perl install.

Thanks,
Chris

@ChrisPortman
ChrisPortman force-pushed the shell branch 2 times, most recently from c6f9c89 to 352e0c5 Compare July 23, 2015 03:07
Chris Portman
Force $SHELL to /bin/bash during install
a8b74bf 
When I did the install, the .perlbrew/init file used `setenv` commands to setup the environment variables.  That command is apparently a tsch equivilent command to export and results in:

-bash: setenv: command not found

Reading through gugod/App-perlbrew#221 , it appears that the installer checks the value of $SHELL to determine which syntax to use.  Add the environment variable to the exec.  This seems to fix it.
Chris Portman added 2 commits July 23, 2015 13:56
Chris Portman
Create a perlbrew::switch define type
bc5001e 
This separates it from the perl install to allow for more flexibility across various use cases.
Chris Portman
Make switching to a new perl optional
088e604 
Add a 'switch' boolean option to perlbrew::perl that allows switching to the new perl after install optional.  Defaults to true to preserve existing implementations.
jhoblitt
jhoblitt reviewed Oct 14, 2015
View reviewed changes
Comment thread manifests/switch.pp Outdated

@jhoblitt jhoblitt Oct 14, 2015

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This param is unvalidated.

@jhoblitt

jhoblitt commented Oct 14, 2015

Copy link
Copy Markdown
Owner

@ChrisPortman Could you take a stab at fixing the tests?

@ChrisPortman

ChrisPortman commented Nov 10, 2015

Copy link
Copy Markdown
Author

Hi Joshua,
Sorry for the delay. I've been away of late.

I changed a few things:

  • "The $version is validated as a string and defaults to 'system' which turns off perlbrew.
  • I've updated the code/tests so that the non-future tests pass. Although, I will admit, I have not increased the coverage, just made sure the existing tests pass.

Thanks,
Chris

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.

2 participants

@ChrisPortman @jhoblitt