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

Standard Credentials File #8

Merged
merged 4 commits into from
Apr 20, 2019
Merged

Standard Credentials File #8

merged 4 commits into from
Apr 20, 2019

Conversation

tedivm
Copy link
Member

@tedivm tedivm commented Apr 13, 2018

This PR adds a standard credentials file to be used by third party applications.

@AlinaNova21
Copy link
Member

As a side thought, most screeps utilities have other options along side the connection, for example, screeps console has max_history, max_scroll, smooth_scroll, etc. Would it be worth adding a configs section that allows each apps config to be specified? These could easily be overridden by a local config for cases such as multiple stats agents.
ex:

servers:
  ...
configs:
  screepsconsole:
    max_history: 20000
    max_scroll: 20000
  screepsplus-agent:
    token: sptoken
    checkForUpdates: false

Updated `connections` to `servers`
Added XDG path to valid paths
Added optional configs section
@Jomik
Copy link

Jomik commented Apr 19, 2019

As I mentioned to @ags131 in slack, I propose something more along the lines of

  • Project Root (Optional) - (project/.screeps.yaml)
  • Current Working Directory - (./.screeps.yaml)
  • Screeps config env - ($SCREEPS_CONFIG)
  • XDG Config Home - (${XDG_CONFIG_HOME:-$HOME/.config}/screeps/config.yaml)
  • Home Directory - (~/.screeps.yaml)

@AlinaNova21
Copy link
Member

AlinaNova21 commented Apr 20, 2019

  • Added the SCREEPS_CONFIG env variable option, I put it first in the list since it should override any other paths.
  • Adjusted $XDG_CONFIG_HOME to use variable rather than ~/.config ,
  • Added %APPDATA% for windows users

IMO, any where the env variable isn't set should be skipped in implementations. Thus the default (${XDG_CONFIG_HOME:-$HOME/.config}) isn't needed

@tedivm
Copy link
Member Author

tedivm commented Apr 20, 2019

@ags131 if you think this is ready to merge lets go ahead and do it.

@Jomik
Copy link

Jomik commented Apr 20, 2019

Well, my only thing is that the spec for xdg says to use ${XDG_CONFIG_HOME:-$HOME/.config}
That is, if it is not set, then fallback to ~/.config, my system does not have the env variable set, and I do use xdg config home..
But I mean, it is rather simple to set, just inconsistent with the spec.

@tedivm
Copy link
Member Author

tedivm commented Apr 20, 2019

Yeah, the xdg spec does have a default for if the environmental variable is set, so it does make sense to fall back on that. I think the clearest way to do that would be to explicitly add the default xdg location to the list (after the one where the environmental variable is set)-

* XDG Config Default Directory - (`$HOME/.config/screeps/config.yaml`)

@AlinaNova21
Copy link
Member

Thats easy enough to add, For windows, should these paths still be checked or just skipped? ($HOME doesn't exist, instead its %USERPROFILE%)

@Jomik
Copy link

Jomik commented Apr 20, 2019

Well, I guess xdg doesn't make sense on Windows though.

@AlinaNova21
Copy link
Member

I'm fine implementing that as just if !Windows, add XDG paths

@AlinaNova21
Copy link
Member

LGTM to merge. @Jomik, any other concerns before merging? (We can always open other PRs later to discuss changes too :) )

@Jomik
Copy link

Jomik commented Apr 20, 2019

I'm happy with it for my uses 😁

@AlinaNova21 AlinaNova21 merged commit b171332 into master Apr 20, 2019
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