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

Adding verify command #387

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

Lite5h4dow
Copy link

@Lite5h4dow Lite5h4dow commented Nov 24, 2021

What does this PR do?

This addition adds the functionality to query local internal yadm commands for the purposes of creating alt files easier.

What issues does this PR fix or reference?

#360

Previous Behavior

There is not previous behavior for this addition

New Behavior

-o/-os : echos the operating system
-d/-distro: echos the distro
-u/-user: echos the user
-c/-class: echos the class
-h/-host: echos the hostname

Have tests been written for this change?

No

Have these commits been signed with GnuPG?

No


Please review yadm's Contributing Guide for best practices.

@Lite5h4dow
Copy link
Author

@TheLocehiliosan hi are you still dealing with personal stuff?

@TheLocehiliosan
Copy link
Member

I've reviewed this PR, and I've a few comments.

First, I don't think that "verify" is the most descriptive command we could use here. Seeing as the command just outputs the requested value, perhaps "echo" would work? Also, perhaps the value itself could be the simple parameter (without the --).

For example:

yadm echo os
yadm echo distro

Next, it could be nice to output ALL the values, if none are specified (for example just running yadm echo). Of course, then labels would also have to be output to make it clear.

For a while I was also torn about implementing an additional command instead of extending the "introspect" command that already exists. For example, I could see adding yadm introspect vars and that would output all of these labeled values. And possibly yadm introspect paths and that would output labeled paths (like repo, worktree, encrypt, etc. basically all the paths mentioned in FILES section of the manage). The functionality of outputting a single unlabeled variable value could be preserved with something like: yadm introspect vars distro?

However, the more I've thought about it, I do want "introspect" to remain focused on outputting details of the yadm program, not the user's environment (I realize yadm introspect repo breaks away from this). So I think I do favor a separate command, that works like "introspect", but outputs labeled values. The best name I can think of right now is "echo", but I'll think more about it.

But I would imagine something like this:

yadm echo vars
yadm variables:
  distro=CentOS
  os=Linux
  hostname=myhost
  etc.

yadm echo vars distro
CentOS

yadm echo paths
yadm paths:
  repo=<path-to-repo>
  yadm_dir=<path-to-yadmdir>
  config=<path-to-yadm-config>
  encrypt=<path-to-encrypt-config>
  archive=<path-to-archive-file>
  etc.

yadm echo paths repo
<path-to-repo>

yadm echo
yadm variables:
  distro=CentOS
  os=Linux
  hostname=myhost
  etc.
yadm paths:
  repo=<path-to-repo>
  yadm_dir=<path-to-yadmdir>
  config=<path-to-yadm-config>
  encrypt=<path-to-encrypt-config>
  archive=<path-to-archive-file>
  etc.

@Lite5h4dow
Copy link
Author

Excellent ill get to work on those changes as soon as i have a chance! thanks for reviewing

@Lite5h4dow
Copy link
Author

just realised that we cant use echo given the cirrent system as echo is a bash keyword, cant name the function echo.

@TheLocehiliosan
Copy link
Member

just realised that we cant use echo given the cirrent system as echo is a bash keyword, cant name the function echo.

echo as a subcommand should be possible, it probably just means you need to adjust the YADM_COMMAND prior to it being called, something like:

YADM_COMMAND="${1/echo/yadm_echo}"

Then the function can be called yadm_echo, but invoked via yadm echo.

@Lite5h4dow
Copy link
Author

@TheLocehiliosan ive integrated the requested changes again, im new to writing bash scripting so if there is a better what to do things let me know. hopefully these are better for you

@Lite5h4dow
Copy link
Author

just realised that we cant use echo given the cirrent system as echo is a bash keyword, cant name the function echo.

echo as a subcommand should be possible, it probably just means you need to adjust the YADM_COMMAND prior to it being called, something like:

YADM_COMMAND="${1/echo/yadm_echo}"

Then the function can be called yadm_echo, but invoked via yadm echo.

This is a much better solution than what I came up with I'll implement this in a sec

@techie2000
Copy link

Having spent a few days 'playing' with yadm, I have to say I would very much welcome this being merged rather than consulting https://yadm.io/docs/alternates# each time :)

@Lite5h4dow
Copy link
Author

Damn, I forgot about this, uh, sure! I use home-manager now tho

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants