Skip to content

feat: Add config-manager push raw command#78

Open
dallinjsevy wants to merge 3 commits into
mainfrom
feature/config-manager-push-raw
Open

feat: Add config-manager push raw command#78
dallinjsevy wants to merge 3 commits into
mainfrom
feature/config-manager-push-raw

Conversation

@dallinjsevy

Copy link
Copy Markdown

No description provided.

@dallinjsevy dallinjsevy force-pushed the feature/config-manager-push-raw branch 2 times, most recently from 8351261 to 4c01c61 Compare March 23, 2026 13:49
@dallinjsevy dallinjsevy marked this pull request as draft March 25, 2026 21:49
@dallinjsevy dallinjsevy force-pushed the feature/config-manager-push-raw branch 2 times, most recently from a480ed2 to 1f98b35 Compare April 2, 2026 15:26
@dallinjsevy dallinjsevy marked this pull request as ready for review April 2, 2026 15:26
@phalestrivir phalestrivir force-pushed the feature/config-manager-push-raw branch from 1f98b35 to 0b4befb Compare April 9, 2026 18:45

@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.

See comments below (note I already rebased this PR with main when I was reviewing it, so you will need to pull those changes before starting on these fixes)

Comment thread rawForgeops.json Outdated
Comment thread rawCloud.json Outdated
Comment thread raw.json Outdated
Comment thread test/e2e/config-manager-push-raw.e2e.test.js
Comment thread test/e2e/config-manager-push-raw.e2e.test.js Outdated
Comment thread src/configManagerOps/FrConfigRawOps.ts Outdated
Comment thread src/configManagerOps/FrConfigRawOps.ts Outdated
Comment thread test/e2e/config-manager-push-raw.e2e.test.js
Comment thread src/cli/config-manager/config-manager-push/config-manager-push-raw.ts Outdated
Comment thread src/cli/config-manager/config-manager-push/config-manager-push-raw.ts Outdated
@dallinjsevy dallinjsevy force-pushed the feature/config-manager-push-raw branch from 0b4befb to 4a6f2c0 Compare April 10, 2026 14:30
@dallinjsevy dallinjsevy force-pushed the feature/config-manager-push-raw branch from 4a6f2c0 to 8f0c6de Compare May 22, 2026 21:42
} else {
delete data._rev;
delete data._type;
await importRawConfig({ path: rawPath }, data);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is rawPath the correct value for path? It seems importRawConfig expects path to be the URL path to the resource rather than the path to the config file.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It looks like it's correct, this is the case when there is only one item at the path as opposed to many. However, the way this is written is pretty confusing. I left a comment suggesting some improvements to simplify this similar to how fr-config-manager does it.

Comment thread src/configManagerOps/FrConfigRawOps.ts
} else {
delete data._rev;
delete data._type;
await importRawConfig({ path: rawPath }, data);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It looks like it's correct, this is the case when there is only one item at the path as opposed to many. However, the way this is written is pretty confusing. I left a comment suggesting some improvements to simplify this similar to how fr-config-manager does it.

Comment thread test/e2e/mocks/github_584874290/recording.har
Comment thread test/e2e/mocks/npmjs_1455397529/recording.har
Comment thread test/e2e/config-manager-push-raw.e2e.test.js Outdated
Comment thread test/e2e/config-manager-push-raw.e2e.test.js Outdated
@dallinjsevy dallinjsevy force-pushed the feature/config-manager-push-raw branch from 8f0c6de to a17bdc2 Compare June 16, 2026 16:34
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.

3 participants