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.

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

Deprecate 'stack path --global-stack-root', add '--stack-root' options – #1148 #1983

Merged
merged 2 commits into from
Apr 12, 2016

Conversation

sjakobi
Copy link
Member

@sjakobi sjakobi commented Apr 1, 2016

No description provided.

@sjakobi
Copy link
Member Author

sjakobi commented Apr 1, 2016

I have added a deprecation warning that shows up when someone uses the stack path --global-stack-root flag explicitely:

$ stack path --global-stack-root
'--global-stack-root' will be removed in a future release.
Please use '--stack-root' instead.

/home/simon/.stack

Should there also be a warning for stack path without any flags? I'm not sure whether it would be obnoxious or maybe even break things. On the other hand people might be confused that stack path lists both global-stack-root and stack-root with the same content:

$ stack path
global-stack-root: /home/simon/.stack
stack-root: /home/simon/.stack
...

@sjakobi
Copy link
Member Author

sjakobi commented Apr 1, 2016

One aspect of #1148 was that the documentation on $STACK_ROOT was somewhat sparse.

I think the situation has already become a bit better with the new entry in stack --help:

  --stack-root STACK-ROOT  Absolute path to the global stack root directory
                           (Overrides any STACK_ROOT environment variable)

Yet should I possibly add an entry to the FAQ, e.g. How can I change the location of the global stack root directory?. Or is there a more appropriate place?

`stack-root` flag and output field. See
`stack-root` flag and output field.
Additionally, the stack root can now be specified via the
`--stack-yaml` command-line flag. See
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be --stack-root, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Of course. Fixed.

@mgsloan
Copy link
Contributor

mgsloan commented Apr 2, 2016

LGTM, thanks! Just one comment that needs to be addressed.

@sjakobi
Copy link
Member Author

sjakobi commented Apr 2, 2016

I have added a commit that fixes an exception for permission-checking code that was still $STACK_ROOT-specific.

@mgsloan: Have you seen my two comments at the top or do you think these are non-issues?

@mgsloan
Copy link
Contributor

mgsloan commented Apr 4, 2016

Should there also be a warning for stack path without any flags? I'm not sure whether it would be obnoxious or maybe even break things. On the other hand people might be confused that stack path lists both global-stack-root and stack-root with the same content:

I think removing it from the list is fine. True, someone could be parsing it. However, I don't know of any tools that do this. The breakage should be minimal at this time.

As long as we're doing such a deprecation / change for --global-stack-root, it'd be good to also address #1546

Yet should I possibly add an entry to the FAQ, e.g. How can I change the location of the global stack root directory?. Or is there a more appropriate place?

Thanks for considering docs! I think the current level of docs is fine. AFAIK there is little need for STACK_ROOT other than having shorter paths on windows, and this is well covered in the windows installation instructions

@sjakobi
Copy link
Member Author

sjakobi commented Apr 6, 2016

I think removing it from the list is fine.

Fixed with edaae29.

As long as we're doing such a deprecation / change for --global-stack-root, it'd be good to also address #1546

I'll take a look!

@sjakobi
Copy link
Member Author

sjakobi commented Apr 11, 2016

Rebased.

@mgsloan
Copy link
Contributor

mgsloan commented Apr 12, 2016

Thanks!

That appveyor failure seems superfluous, so merging.

@mgsloan mgsloan merged commit afead43 into commercialhaskell:master Apr 12, 2016
@sjakobi sjakobi deleted the 1148-stack-root branch April 12, 2016 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants