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

Normalize screaming case html yaml for zeitwerk #22489

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented May 2, 2023

Extracted from the zeitwerk PR

Normalize on Camelcase Html for consistency with loading

We need to match MiqReport::Formatters:Html and MiqReport::Generator::Html or
change them to also be HTML. I decided on Html because HTML would also require
us to teach zeitwerk another acronym.

Use Yaml as YAML looks awful in YAMLImportExportMixin

Acronym make sense when the change to use Camelcase is hard or it reads better.
We already use Yaml in constant names so it makes sense to convert this and get
rid of YAML. Then, we also don't need to define an acronym.

Acronym make sense when the change to use Camelcase is hard or it reads better.
We already use Yaml in constant names so it makes sense to convert this and get
rid of YAML.  Then, we also don't need to define an acronym.
We need to match MiqReport::Formatters:Html and MiqReport::Generator::Html or
change them to also be HTML.  I decided on Html because HTML would also require
us to teach zeitwerk another acronym.
@miq-bot
Copy link
Member

miq-bot commented May 2, 2023

Checked commits jrafanie/manageiq@14a845f~...9e0de20 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
12 files checked, 1 offense detected

lib/manageiq/reporting/formatter.rb

  • ❗ - Line 34, Col 22 - Performance/StringIdentifierArgument - Use :ReportHTML instead of 'ReportHTML'.

@jrafanie
Copy link
Member Author

jrafanie commented May 2, 2023

@miq-bot cross-repo-test manageiq-ui-classic

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request May 2, 2023
@kbrock kbrock self-assigned this May 3, 2023
@kbrock kbrock added the rails7 label May 3, 2023
@kbrock
Copy link
Member

kbrock commented May 3, 2023

verified that I couldn't find any references to the 2 screamers

@kbrock kbrock merged commit 31dae8f into ManageIQ:master May 3, 2023
@kbrock kbrock deleted the normalize_screaming_case_html_yaml_for_zeitwerk branch May 3, 2023 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants