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

Delete default configuration #49

Merged
merged 24 commits into from
Nov 15, 2021

Conversation

fluca1978
Copy link
Collaborator

See #47.

Now there are different config subcommands:

  • config init creates always a blank configuration file
    • config write creates a configuration file with the default configuration if exists, or a blank file otherwise
    • config delete does not delete the default configuration file unless explicitly indicated as config delete default
    • config nuke deletes every configuration specified, even the default if not passed as argument.

Use `date` to place a timestamp like string to configuration backup files,
created when overwriting an existing configuration file.
`config init` will create a new existing file with blank settings.
See <theory#47 (comment)>
Since now the configuration files include the timestamp string,
the `config delete` command must delete everything that ends in `.backup`.
`config nuke` does the same as `config delete` but allows the user to delete
the default configuration file. Now `config delete` does not allow for
removal of the default configuration file, but `config nuke` allows for
the removal of the default configuration file.

Specifying `config delete default` or `config nuke` will provide the same
behavior, that is deletion of the default configuration.

See <theory#47 (comment)>
Now the command loads the current default configuration and
dumps it as the specified file.
Introduction to the new behavior of the `config` subcommands.

See <theory#47 (comment)>
@theory
Copy link
Owner

theory commented Nov 8, 2021

Shouldn't config write write out the interpreted configuration?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
bin/pgenv Show resolved Hide resolved
Check for the existance of the configuration file when `config init` is run
and avoid re-doing initialization then.
The latter is enough explicit to warn the user to do the deletion.
Remove the `config nuke` references.
Explain the workflow of `config init`.
@fluca1978
Copy link
Collaborator Author

I've fixed init so that it does not allows for re-initialization of existing configuration files.
I've also removed config nuke, leaving config delete default as an explicit way. In this way code should be simpler.

I need to rebase on other changes before this is ready.

bin/pgenv Outdated Show resolved Hide resolved
Placed also a debug message to explain which version was in use to double
check the program has "guessed" the right file.
When trying to delete the default configuration by not specifying the
version, the warning message provides more details.
Also removed a reference to `nuke`.
Fix documentation.

Also, example with a serious editor ;)
Started by theory, removed everywhere for coherence.
@fluca1978
Copy link
Collaborator Author

Seems fine to me, I've run a "normal" pgenv build and configure cycle.
Any comment?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
bin/pgenv Show resolved Hide resolved
bin/pgenv Outdated Show resolved Hide resolved
bin/pgenv Outdated Show resolved Hide resolved
bin/pgenv Show resolved Hide resolved
Applied 'sh' and '$' prompt coherently across all the examples.
Fixed a few markdown lists here and there.
Removed the timestamp from the log outputs.
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
Use 'sh' only if there are commands without output, use '$' as prompt only
when there is an user input and output.
Copy link
Owner

@theory theory left a comment

Choose a reason for hiding this comment

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

LGTM. I might tweak some of the code samples and wording once you merge, if that's okay with you.

@fluca1978 fluca1978 merged commit e46d35f into theory:master Nov 15, 2021
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.

None yet

2 participants