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.

Sign up for GitHub

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

Jump to bottom

[Global] Update strings to align with sos tooling names #3666

Merged
merged 1 commit into from
Jun 24, 2024
Merged

[Global] Update strings to align with sos tooling names #3666

merged 1 commit into from
Jun 24, 2024

Conversation

TurboTurtle
Copy link
Member

This commit serves to reinforce the typing of "sos (space) report" in place of the legacy "sosreport", and similarly updates items such as "sos-collector" to "sos collect". The tool is sos ("ess-oh-ess"), of which report and collect are sub-commands.

The most visible change is the update to the line printed before the path to a generate sos report, which may be significant for scripts and/or automation. To be clear, this commit changes this line:

Your sosreport has been generated and saved in:

to

Your sos report has been generated and saved in:

Note that this does not change the use of sosreport when used as part of method names, nor does it change the filename of the generated archive for either report or collect.

Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

  • Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • Is the subject and message clear and concise?
  • Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • Does the commit contain a Signed-off-by: First Lastname email@example.com?
  • Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
  • Are all passwords or private data gathered by this PR obfuscated?

@packit-as-a-service Packit-as-a-Service
Copy link

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/sosreport-sos-3666
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@arif-ali
Copy link
Member

What about changing the man pages?

Thoughts on replacing sosreport with sos report in sos/collector/__init__.py where we have the following lines (from grep); or do we want to keep backward compatibility

sos/collector/__init__.py:        sos_cmd = 'sosreport --batch '
sos/collector/__init__.py:        self.commons['sos_cmd'] = 'sosreport --batch '

@pmoravec
Copy link
Contributor

I think sos-collect can technically call sosreport in version 3.* from a legacy system, but.. would that system be in a cluster with sos 4.8+ ?

Similar point to sosreport binary applies to sos/collector/sosnode.py as well. We can leave it there for legacy reasons, though I think it wont be ever used.

man/en/sos.conf.5 has three mentions of sosreport, but it is questionable where it means sosreport as file, sosreport as the project or sos as the program.

@TurboTurtle
Copy link
Member Author

Didn't do the man pages yet, but would probably be good to include that in this change after all.

Thoughts on replacing sosreport with sos report in sos/collector/init.py where we have the following lines (from grep); or do we want to keep backward compatibility

Similar point to sosreport binary applies to sos/collector/sosnode.py as well. We can leave it there for legacy reasons, though I think it wont be ever used.

Yeah, leaving that there for legacy support currently. I think we would drop that at the same time we eventually drop the bin/sosreport and bin/sos-collector redirectors we currently carry. On that note, would we all be ok with adding a warning with the upcoming 4.8 release (August) that says "this redirector will be removed in the next minor release", and then drop it in 4.9?

@pmoravec
Copy link
Contributor

+1, worth adding a warning to be nice to users (for two releases / one year maybe? or just 1 release / 6m?), and drop it altogether with the binary redirection.

@arif-ali
Copy link
Member

+1 on deprecation warning and then getting rid; gives us time to update our internal documentation

@TurboTurtle
Copy link
Member Author

Updated with a new warning and removal date for the redirectors, also updated the man pages.

pmoravec
pmoravec reviewed Jun 21, 2024
View reviewed changes
bin/sosreport
Comment on lines +25 to +27
msg = ("WARNING: the 'sosreport' command has been deprecated in favor "
"of the new 'sos' command, E.G. 'sos report', and will be removed "
"in the upcoming sos-4.9 release.\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a similar text to man/en/sosreport.1 as well? man sosreport makes user to still feel nothing is changing..

Copy link
Member Author

Choose a reason for hiding this comment

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

sosreport.1 is a symlink to man/en/sos-report.1, so I hadn't considered adding something there. Could be worth a note in the description though for sure.

@pmoravec
Copy link
Contributor

Sounds good otherwise!

@TurboTurtle
[Global] Update strings to align with sos tooling names
f65ff7f 
This commit serves to reinforce the typing of "sos (space) report" in
place of the legacy "sosreport", and similarly updates items such as
"sos-collector" to "sos collect". The tool is `sos` ("ess-oh-ess"), of
which `report` and `collect` are sub-commands.

The most visible change is the update to the line printed before the
path to a generate sos report, which may be significant for scripts
and/or automation. To be clear, this commit changes this line:

`Your sosreport has been generated and saved in:`

to

`Your sos report has been generated and saved in:`

Note that this does not change the use of `sosreport` when used as part
of method names, nor does it change the *filename* of the generated
archive for either `report` or `collect`.

Signed-off-by: Jake Hunsaker <jacob.r.hunsaker@gmail.com>
@TurboTurtle
Copy link
Member Author

Updated with a new note in the manpages about the deprecation.

@TurboTurtle TurboTurtle merged commit e48a41d into sosreport:main Jun 24, 2024
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arif-ali arif-ali arif-ali approved these changes

@pmoravec pmoravec pmoravec approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@TurboTurtle @arif-ali @pmoravec