Skip to content

Conversation

@bocekm
Copy link
Member

@bocekm bocekm commented Nov 20, 2024

Env vars have been deprecated in favor of the ini config file (e.g. /etc/convert2rhel.ini). We should be telling users to use the config file instead of the deprecated env vars.

Jira Issues:

Checklist

  • PR has been tested manually in a VM (either author or reviewer)
  • Jira issue has been made public if possible
  • [RHELC-] or [HMS-] is part of the PR title
  • Label depicting the kind of PR it is
  • PR title explains the change from the user's point of view
  • Code and tests are documented properly
  • The commits are squashed to as few commits as possible (without losing data)
  • When merged: Jira issue has been updated to Release Pending if relevant

@bocekm bocekm requested a review from danmyway November 20, 2024 15:58
@bocekm bocekm added skip/changelog If it should be excluded from changelog or Release notes. Such as infra, reverted PRs, etc. tests/sanity PR ready to run the sanity test suit. Equivalent to `/packit test --labels sanity`. kind/refactor labels Nov 20, 2024
@has-bot
Copy link
Member

has-bot commented Nov 20, 2024

/packit test --labels sanity


Comment generated by an automation.

@bocekm bocekm force-pushed the messages-to-tell-to-use-config-not-env-vars branch from 2b03ee6 to 20c620c Compare November 20, 2024 23:37
@danmyway
Copy link
Member

/packit test

@bocekm bocekm changed the title [RHELC-1511] Tell user to use config ini instead of env vars [RHELC-1511] Tell users to use config ini instead of env vars Nov 21, 2024
@bocekm bocekm force-pushed the messages-to-tell-to-use-config-not-env-vars branch from 9a90f0d to e235cbd Compare November 21, 2024 19:50
@danmyway
Copy link
Member

/packit test

@bocekm bocekm force-pushed the messages-to-tell-to-use-config-not-env-vars branch from 58b50cd to b737455 Compare November 22, 2024 14:44
@bocekm bocekm marked this pull request as ready for review November 22, 2024 14:44
@bocekm bocekm requested a review from a team as a code owner November 22, 2024 14:44
@bocekm bocekm force-pushed the messages-to-tell-to-use-config-not-env-vars branch from 68e08e4 to 913b709 Compare November 22, 2024 15:43
@codecov
Copy link

codecov bot commented Nov 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.09%. Comparing base (c83d7b6) to head (3caee03).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1433      +/-   ##
==========================================
- Coverage   96.09%   96.09%   -0.01%     
==========================================
  Files          72       72              
  Lines        5143     5142       -1     
  Branches      888      888              
==========================================
- Hits         4942     4941       -1     
  Misses        119      119              
  Partials       82       82              
Flag Coverage Δ
centos-linux-7 91.57% <100.00%> (-0.01%) ⬇️
centos-linux-8 92.44% <100.00%> (-0.01%) ⬇️
centos-linux-9 92.56% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bocekm
Copy link
Member Author

bocekm commented Nov 25, 2024

I can split this up into necessary-only changes and nice to have formatting changes so that there's not that many changes to review at once.

@bocekm bocekm force-pushed the messages-to-tell-to-use-config-not-env-vars branch from 3619535 to b0e1b95 Compare November 25, 2024 21:53
@bocekm
Copy link
Member Author

bocekm commented Nov 25, 2024

Ok, I have slashed the number of changed lines to a third, keeping just the bare minimum. I'll create another PR for the nice-to-have changes to the messages.
EDIT: Here it is: #1438

@bocekm bocekm force-pushed the messages-to-tell-to-use-config-not-env-vars branch 2 times, most recently from 0db29cc to 34af479 Compare November 26, 2024 16:45
@bocekm bocekm requested a review from a team as a code owner November 26, 2024 16:45
@bocekm bocekm force-pushed the messages-to-tell-to-use-config-not-env-vars branch from 34af479 to 539280d Compare November 26, 2024 17:55
@danmyway
Copy link
Member

/packit test

@bocekm bocekm added tests/tier0 PR ready to run the essential test suit. Equivalent to `/packit test --labels tier0`. and removed tests/sanity PR ready to run the sanity test suit. Equivalent to `/packit test --labels sanity`. labels Nov 26, 2024
@has-bot
Copy link
Member

has-bot commented Nov 26, 2024

/packit test --labels tier0


Comment generated by an automation.

Env vars have been deprecated in favor of the ini config file (e.g.
/etc/convert2rhel.ini). for that reason We should be telling users to use
the config file instead of the deprecated env vars.
@danmyway danmyway force-pushed the messages-to-tell-to-use-config-not-env-vars branch from 539280d to 3caee03 Compare November 27, 2024 10:32
@danmyway
Copy link
Member

/packit test

Copy link
Member

@danmyway danmyway left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for taking care of the tests.

@danmyway danmyway merged commit 40f5d6e into oamg:main Nov 27, 2024
46 of 53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/refactor skip/changelog If it should be excluded from changelog or Release notes. Such as infra, reverted PRs, etc. tests/tier0 PR ready to run the essential test suit. Equivalent to `/packit test --labels tier0`.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants