Skip to content

refactor: realm flag removal for config-manager - #116

Open
brycentrivir wants to merge 10 commits into
mainfrom
bugfix/remove-config-manager-realms
Open

refactor: realm flag removal for config-manager#116
brycentrivir wants to merge 10 commits into
mainfrom
bugfix/remove-config-manager-realms

Conversation

@brycentrivir

Copy link
Copy Markdown

Removal of -r, --realm flag throughout config-manager pull commands as there was a redundancy created as Frodo already has the realm argument built in.

@brycentrivir
brycentrivir marked this pull request as ready for review June 25, 2026 17:32
@brycentrivir brycentrivir changed the title bugfix: realm flag removed bugfix: realm flag removed for config-manager Jun 25, 2026

@phalestrivir phalestrivir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

After the changes, make sure to update the tests accordingly. Main thing to remember is that for those commands that due use the realm argument we need to have a separate test where we specify the realm that way, which it looks like you already did, but I won't look over the tests until these changes are done since the tests I know are going to change for a lot of these. For example, all-static has the realm test that you added which is good, but we never did implement the realm argument for it (which I left a comment about), so once you implement that you will need to re-record that test since it currently doesn't test anything.

Comment thread src/cli/config-manager/config-manager-pull/config-manager-pull-authz-policies.ts Outdated
Comment thread src/cli/config-manager/config-manager-pull/config-manager-pull-authz-policies.ts Outdated
Comment thread src/cli/config-manager/config-manager-pull/config-manager-pull-authz-policies.ts Outdated
Comment thread src/cli/config-manager/config-manager-pull/config-manager-pull-org-privileges.ts Outdated
Comment thread src/cli/config-manager/config-manager-pull/config-manager-pull-scripts.ts Outdated
Comment thread src/configManagerOps/FrConfigOrgPrivilegesOps.ts Outdated
@brycentrivir
brycentrivir force-pushed the bugfix/remove-config-manager-realms branch 2 times, most recently from a955322 to 5fdd5c2 Compare July 28, 2026 21:03
@brycentrivir
brycentrivir force-pushed the bugfix/remove-config-manager-realms branch from 6cd062d to 5f23d6f Compare August 5, 2026 21:40
@brycentrivir brycentrivir changed the title bugfix: realm flag removed for config-manager refactor: realm flag removal for config-manager Aug 13, 2026
@brycentrivir
brycentrivir force-pushed the bugfix/remove-config-manager-realms branch from eb98657 to 2c89667 Compare August 13, 2026 16:44
@brycentrivir

Copy link
Copy Markdown
Author

I've made the changes I think are good for review for now. The all-static is not complete, I need to know what functions apply to that one. I can record tests properly once I know that.

@phalestrivir

Copy link
Copy Markdown

I've made the changes I think are good for review for now. The all-static is not complete, I need to know what functions apply to that one. I can record tests properly once I know that.

I added some notes to this clickup article on which configurations are static: https://app.clickup.com/45049292/v/dc/1aytec-23271/1aytec-64011

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

Successfully merging this pull request may close these issues.

2 participants