Skip to content

Remove note about config paths - #6516

Merged
bsipocz merged 1 commit into
astropy:masterfrom
saimn:config-doc
Sep 7, 2017
Merged

bsipocz merged 1 commit into
astropy:masterfrom
saimn:config-doc

Conversation

@saimn

@saimn saimn commented Sep 5, 2017

Copy link
Copy Markdown
Contributor

Follow up to #6512, see comments there for the motivation: this paragraph is confusing as it follows another paragraph on the Astropy default config path. And the information given here is not really useful I think.
cc @pllim @bsipocz

@astropy-bot

astropy-bot Bot commented Sep 5, 2017 •

Copy link
Copy Markdown

Hi there @saimn 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labelled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃.

Everything looks good from my point of view! 👍

@pllim

pllim commented Sep 5, 2017

Copy link
Copy Markdown
Member

Is it worth replacing it with a less confusing note that XDG_CONFIG_HOME is also used by other software and then give a link to, say, https://wiki.archlinux.org/index.php/XDG_Base_Directory_support (or something more general) for further reading?

@pllim

pllim commented Sep 5, 2017

Copy link
Copy Markdown
Member

And is this XDG_CONFIG_HOME supported under Windows? It is not clear from the documentation.

@saimn

saimn commented Sep 5, 2017

Copy link
Copy Markdown
Contributor Author

@pllim - Good point, XDG is a linux thing (the X Desktop Group), but the code handles this independently from the OS.

@saimn

saimn commented Sep 5, 2017

Copy link
Copy Markdown
Contributor Author

@pllim - I tried to explain a bit more XDG_CONFIG_HOME, is it better like this ?

@pllim pllim left a comment

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.

This is much better. Thanks, @saimn !

Should let the CIs run though (no [skip ci]) to make sure Sphinx gives no warning.

Comment thread docs/config/index.rst Outdated
configuration in ``~/.astropy/config``.
scheme generally puts your configuration directory in
``$HOME/.astropy/config``. It can be customized with the environment variable
``XDG_CONFIG_HOME`` and the ``$XDG_CONFIG_HOME/astropy`` directory must exists.

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 thought "must exist" is more grammatically correct but I am not 100% sure.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok thanks, I will update this and squash.

@bsipocz

bsipocz commented Sep 7, 2017

Copy link
Copy Markdown
Member

Passed (except the osx build that I've cancelled), so I'm merging now. Thanks @saimn!

@bsipocz
bsipocz merged commit 22102d0 into astropy:master Sep 7, 2017
@saimn
saimn deleted the config-doc branch September 7, 2017 08:09
bsipocz added a commit that referenced this pull request Sep 8, 2017
Remove note about config paths
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants