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

RFC: Remove the '...' at the end of the effective configuration output #26

Open
ebugden opened this issue Feb 14, 2023 · 7 comments
Open

Comments

@ebugden
Copy link
Contributor

ebugden commented Feb 14, 2023

The "show-effective-config" command prints the entire configuration object that will be used by barectf, however the '...' that appears at the end of the output implies continuation. It suggests the output was cropped or is incomplete.

In my case, after using the command I went to check the YAML configuration file to make sure the end of the command output matched the end of the file.

Remove the '...' to clarify that the command outputs the complete configuration object.

image

@eepp
Copy link
Member

eepp commented Feb 14, 2023

This is valid YAML; see Document Markers.

Three dots means "end of document" and confirms that it's complete and nothing more is to be expected.

@ebugden
Copy link
Contributor Author

ebugden commented Feb 14, 2023

Ah ok! Is there a functional reason for displaying the "document end marker" at the end of the effective configuration output? My understanding is that the document markers are to avoid ambiguity about where files start/end when parsing multiple YAML files in sequence: "YAML uses three dashes (“ --- ”) to separate documents within a stream. Three dots ( “ ... ”) indicate the end of a document without starting a new one, for use in communication channels.", but in this case the command's goal seems to only be to clearly communicate the configuration object to the user.

My impression is that in practice '...' aren't often used in YAML files (some quick sources: (1) most yaml tutorials don't seem to include them in their file examples, (2) comment, (3) @mjeanson has never seen this end marker in a YAML file before) and that most users won't understand that '...' explicitly indicates the end of a file.

If the command goal is to clearly communicate the configuration to the average user, if it's quick and easy to remove '...' from the output, and if this change has no negative functional impact on barectf, I think removing it would make things more clear for most folks using the command.

That being said, here are some caveats that could make this change less relevant:

  • If it's important that users understand what '...' means for properly writing config files for barectf (e.g. For writing config files that will be included in another config)
  • If the effective config command is only used very infrequently and by people who would recognize the document marker
  • If the effective config command is used so infrequently that this won't confuse many people and isn't worth the effort to change

@eepp
Copy link
Member

eepp commented Feb 14, 2023

No functional reason!

I believe I just opted for the most complete YAML document.

If:

  • It's useless in that context.
  • You can confirm that the printed output is still valid without the dots.
  • It's important for you not to have said dots.

Then I'll accept a Gerrit change for the ˋmaster` branch.

@ebugden
Copy link
Contributor Author

ebugden commented Feb 14, 2023

Sounds good! How would you like me to confirm that it's useless? That the project tests pass? That this command output is unlikely to be used in scripts that this change could break? I can also make a reasonable guess and you can let me know later if I missed something.

Also what do you mean by "still valid without the dots"? That the output is a valid YAML file? (Is a goal also that users can copy-paste the output of that command into a file and use that as their config?)

@eepp
Copy link
Member

eepp commented Feb 16, 2023

Sounds good! How would you like me to confirm that it's useless? That the project tests pass? That this command output is unlikely to be used in scripts that this change could break? I can also make a reasonable guess and you can let me know later if I missed something.

I guess removing those dots could break something somewhere, but I'm confident that it won't. So I'd just do it and wait for bug reports which will probably never exist.

Also what do you mean by "still valid without the dots"? That the output is a valid YAML file? (Is a goal also that users can copy-paste the output of that command into a file and use that as their config?)

Yes that's the purpose of this command, especially when reading a barectf 2 config.

@ebugden
Copy link
Contributor Author

ebugden commented Feb 22, 2023

ebugden: Is a goal also that users can copy-paste the output of that command into a file and use that as their config?

eepp: Yes that's the purpose of this command, especially when reading a barectf 2 config.

Oh! So would the main purposes of this command be:

  • To see the full expanded config (with inclusions and hierarchy applied) to more easily review the config
  • To convert a v2 config to a valid v3 config (so that the v2 config file can be more easily replaced)

Asking so that I don't propose changes that break the intended use cases.

@eepp
Copy link
Member

eepp commented Feb 22, 2023

Yes exactly.

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

No branches or pull requests

2 participants