-
Notifications
You must be signed in to change notification settings - Fork 0
Revamp CLI docs, improve commands, add vnextConfig #5
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
Revamp CLI docs, improve commands, add vnextConfig #5
Conversation
Major update to CLI documentation in README.md, including detailed usage, command explanations, and vnext.config.json support. Adds new src/lib/vnextConfig.js for reading project config, improves logging and user feedback in check, csx, reset, and sync commands, and updates package metadata. Enhances component discovery, error handling, and summary reporting for all major commands.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideRevamps the CLI’s configuration and component model around a required vnext.config.json file, updates all major commands (check, csx, sync, reset, update) to use that config and provide richer logging and summaries, refactors discovery/CSX/workflow logic to be path-aware and safer, and refreshes the README and package metadata accordingly. Sequence diagram for wf update component update flowsequenceDiagram
actor Dev
participant CLI as wf_update_command
participant ConfigLib as Config
participant VnextConfig as VnextConfig
participant Discover as Discover
participant Csx as CsxLib
participant Workflow as WorkflowLib
participant Db as DbLib
participant Api as ApiLib
participant API as Vnext_API
participant DB as Postgres_DB
Dev->>CLI: wf update [--all|--file]
CLI->>ConfigLib: get(PROJECT_ROOT)
ConfigLib-->>CLI: projectRoot (cwd)
CLI->>VnextConfig: getDomain(projectRoot)
VnextConfig-->>CLI: domain
CLI->>ConfigLib: get(API_BASE_URL, API_VERSION)
ConfigLib-->>CLI: apiConfig
alt options.all
CLI->>Csx: findAllCsx(projectRoot)
else default
CLI->>Csx: getGitChangedCsx(projectRoot)
end
Csx-->>CLI: csxFiles[]
loop for each csxFile
CLI->>Csx: processCsxFile(csxPath, projectRoot)
Csx->>Discover: discoverComponents(projectRoot)
Discover-->>Csx: discoveredComponents
Csx->>Discover: findAllJsonFiles(discoveredComponents)
Csx-->>CLI: {updatedJsonCount, totalUpdates}
end
alt options.file
CLI->>Workflow: getJsonMetadata(filePath)
Workflow-->>CLI: metadata
else options.all
CLI->>Discover: discoverComponents(projectRoot)
Discover-->>CLI: discovered
CLI->>Workflow: findAllJson(discovered)
Workflow-->>CLI: jsonFiles[]
else default
CLI->>Workflow: getGitChangedJson(projectRoot)
Workflow-->>CLI: jsonFiles[]
end
loop for each jsonFile
CLI->>Workflow: getJsonMetadata(jsonPath)
Workflow-->>CLI: {key, version, data}
CLI->>Workflow: detectComponentType(jsonPath, projectRoot)
Workflow-->>CLI: flowType
CLI->>Db: getInstanceId(dbConfig, flowType, key, version)
Db-->>CLI: existingId?
alt exists
CLI->>Db: deleteWorkflow(dbConfig, flowType, existingId)
end
CLI->>Api: publishComponent(apiBaseUrl, data)
Api->>API: POST /api/v1/definitions/publish
API-->>Api: {success|error}
Api-->>CLI: result
end
opt anySuccess
CLI->>Api: reinitializeSystem(apiBaseUrl, apiVersion)
Api->>API: GET /api/v1/definitions/re-initialize
API-->>Api: ok
Api-->>CLI: true|false
end
CLI-->>Dev: summary (per type created/updated/failed, CSX stats)
Class diagram for vnextConfig-centred librariesclassDiagram
class ConfigLib {
+get(key)
+set(key, value)
+getAll()
+clear()
+path
}
class VnextConfigLib {
+loadVnextConfig(projectRoot)
+getDomain(projectRoot)
+getPaths(projectRoot)
+getComponentsRoot(projectRoot)
+getComponentTypes(projectRoot)
+getFullConfig(projectRoot)
+clearCache()
}
class DiscoverLib {
+discoverComponents(projectRoot)
+findJsonInComponent(componentDir)
+findAllJsonFiles(discovered)
+findAllCsxInComponents(projectRoot)
+getComponentDir(discovered, component)
+listDiscovered(discovered, componentTypes)
+detectComponentTypeFromPath(filePath, componentTypes)
}
class CsxLib {
+encodeToBase64(csxPath)
+readNativeContent(csxPath)
+findJsonFilesForCsx(csxPath, projectRoot)
+getCsxLocation(csxPath, projectRoot)
+updateCodeInJson(jsonPath, csxLocation, base64Code, nativeCode)
+processCsxFile(csxPath, projectRoot)
+getGitChangedCsx(projectRoot)
+findAllCsx(projectRoot)
}
class WorkflowLib {
+getJsonMetadata(jsonPath)
+detectComponentType(jsonPath, projectRoot)
+processComponent(jsonPath, dbConfig, baseUrl, projectRoot)
+getGitChangedJson(projectRoot)
+findAllJsonInComponent(componentDir)
+findAllJson(discovered)
}
class ApiLib {
+testApiConnection(baseUrl)
+publishComponent(baseUrl, componentData)
+reinitializeSystem(baseUrl, version)
}
class DbLib {
+testDbConnection(dbConfig)
+getInstanceId(dbConfig, flow, key, version)
+deleteWorkflow(dbConfig, flow, instanceId)
}
class CheckCommand {
+checkCommand()
}
class CsxCommand {
+csxCommand(options)
}
class SyncCommand {
+syncCommand()
}
class ResetCommand {
+resetCommand(options)
}
class UpdateCommand {
+updateCommand(options)
}
ConfigLib <.. CheckCommand
ConfigLib <.. CsxCommand
ConfigLib <.. SyncCommand
ConfigLib <.. ResetCommand
ConfigLib <.. UpdateCommand
VnextConfigLib <.. CheckCommand
VnextConfigLib <.. CsxCommand
VnextConfigLib <.. SyncCommand
VnextConfigLib <.. ResetCommand
VnextConfigLib <.. UpdateCommand
DiscoverLib <.. CheckCommand
DiscoverLib <.. SyncCommand
DiscoverLib <.. UpdateCommand
DiscoverLib <.. CsxLib
DiscoverLib <.. WorkflowLib
CsxLib <.. CsxCommand
CsxLib <.. SyncCommand
CsxLib <.. UpdateCommand
WorkflowLib <.. SyncCommand
WorkflowLib <.. ResetCommand
WorkflowLib <.. UpdateCommand
ApiLib <.. CheckCommand
ApiLib <.. SyncCommand
ApiLib <.. ResetCommand
ApiLib <.. UpdateCommand
DbLib <.. SyncCommand
DbLib <.. ResetCommand
DbLib <.. UpdateCommand
VnextConfigLib ..> DiscoverLib : providesPaths
VnextConfigLib ..> WorkflowLib : providesComponentTypes
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello @yilmaztayfun, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a foundational change to how the CLI manages project configurations by implementing a Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- The LOG helper (separator/header/component/etc.) is duplicated across multiple command files; consider centralizing this in a shared utility (e.g., lib/log.js) to avoid repetition and keep formatting changes consistent.
- There are a few imports that appear unused after the refactor (e.g., findAllJsonFiles in update.js, detectComponentTypeFromPath or related helpers in discover/workflow); a quick pass to remove unused symbols would simplify the modules and reduce cognitive load.
- publishComponent now only uses baseUrl and a fixed /api/v1/definitions/publish route while several callers still construct apiConfig with version/domain fields; you might want to either remove those unused fields or wire them into the API client to keep configuration semantics consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The LOG helper (separator/header/component/etc.) is duplicated across multiple command files; consider centralizing this in a shared utility (e.g., lib/log.js) to avoid repetition and keep formatting changes consistent.
- There are a few imports that appear unused after the refactor (e.g., findAllJsonFiles in update.js, detectComponentTypeFromPath or related helpers in discover/workflow); a quick pass to remove unused symbols would simplify the modules and reduce cognitive load.
- publishComponent now only uses baseUrl and a fixed /api/v1/definitions/publish route while several callers still construct apiConfig with version/domain fields; you might want to either remove those unused fields or wire them into the API client to keep configuration semantics consistent.
## Individual Comments
### Comment 1
<location> `src/lib/csx.js:55-56` </location>
<code_context>
-function getCsxLocation(csxPath) {
- // ./src/Rules/MyRule.csx formatına çevir
- const parts = csxPath.split('/');
+function getCsxLocation(csxPath, projectRoot) {
+ // Convert to ./src/Rules/MyRule.csx format
+ const parts = csxPath.split(path.sep);
const srcIndex = parts.lastIndexOf('src');
</code_context>
<issue_to_address>
**issue (bug_risk):** The new `projectRoot` parameter in `getCsxLocation` is unused and the implementation still hardcodes a `src`-based heuristic.
`projectRoot` is accepted but never used; the function still relies on finding the last `src` segment to build the path. If components are no longer guaranteed to live under `src`, this resolution can fail. Either drop `projectRoot` from the signature, or update the logic to resolve relative to the configured components root (e.g. via `getComponentsRoot`) so it matches the vnext.config-driven layout.
</issue_to_address>
### Comment 2
<location> `src/lib/csx.js:101` </location>
<code_context>
- obj.code = base64Code;
- return true;
+ // Check encoding type: NAT = Native (plain text), B64 or empty = Base64 (default)
+ const encoding = obj.encoding ? obj.encoding.toUpperCase() : 'B64';
+
+ if (encoding === 'NAT') {
</code_context>
<issue_to_address>
**issue (bug_risk):** Encoding handling assumes `encoding` is a string, which can throw if the field is present but non‑string.
Since this parses arbitrary JSON, `obj.encoding.toUpperCase()` can throw when `encoding` is present but not a string (boolean, number, object, etc.). Please add a type guard, e.g.:
```js
const encoding = typeof obj.encoding === 'string'
? obj.encoding.toUpperCase()
: 'B64';
```
This preserves current behavior for valid strings while avoiding runtime errors on malformed data.
</issue_to_address>
### Comment 3
<location> `src/lib/config.js:29-38` </location>
<code_context>
+ */
+function get(key) {
+ // PROJECT_ROOT is always the current working directory
+ if (key === 'PROJECT_ROOT') {
+ return process.cwd();
+ }
+ return config.get(key);
+}
+
+/**
+ * Sets a config value
+ * PROJECT_ROOT cannot be set (ignored)
+ * @param {string} key - Config key
+ * @param {any} value - Config value
+ */
+function set(key, value) {
+ // PROJECT_ROOT cannot be saved - always uses cwd
+ if (key === 'PROJECT_ROOT') {
+ console.log('Note: PROJECT_ROOT is always the current working directory and cannot be changed.');
+ return;
+ }
</code_context>
<issue_to_address>
**suggestion:** Having `config.set` log to the console for `PROJECT_ROOT` introduces a side effect in a library API that might be used programmatically.
This `console.log` causes unexpected stdout output for any code that calls `config.set('PROJECT_ROOT', ...)`, which is problematic for scripts and tests. To avoid side effects in the library API, consider either throwing for this key or silently ignoring it, and move any explanatory messaging to the CLI layer (e.g. `configCommand`) where stdout output is expected.
Suggested implementation:
```javascript
function get(key) {
// PROJECT_ROOT is always the current working directory
if (key === 'PROJECT_ROOT') {
// PROJECT_ROOT is always derived from the current working directory
return process.cwd();
}
return config.get(key);
```
You’ll also want to adjust the `set` implementation in this same file to avoid console side effects:
1. Find the `set` function that currently looks like:
```js
function set(key, value) {
// PROJECT_ROOT cannot be saved - always uses cwd
if (key === 'PROJECT_ROOT') {
console.log('Note: PROJECT_ROOT is always the current working directory and cannot be changed.');
return;
}
// ... probably existing logic here ...
}
```
2. Change it to silently ignore attempts to set `PROJECT_ROOT`:
```js
function set(key, value) {
// PROJECT_ROOT cannot be saved - always uses cwd
if (key === 'PROJECT_ROOT') {
// Silently ignore attempts to set PROJECT_ROOT from library code
return;
}
return config.set(key, value);
}
```
This removes the stdout side effect from the library API while still preventing `PROJECT_ROOT` from being set. If your existing `set` logic differs, keep the existing behavior for non-`PROJECT_ROOT` keys and only replace the `console.log` branch with the early `return`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if (key === 'PROJECT_ROOT') { | ||
| return process.cwd(); | ||
| } | ||
| return config.get(key); | ||
| } | ||
|
|
||
| /** | ||
| * Sets a config value | ||
| * PROJECT_ROOT cannot be set (ignored) | ||
| * @param {string} key - Config key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Having config.set log to the console for PROJECT_ROOT introduces a side effect in a library API that might be used programmatically.
This console.log causes unexpected stdout output for any code that calls config.set('PROJECT_ROOT', ...), which is problematic for scripts and tests. To avoid side effects in the library API, consider either throwing for this key or silently ignoring it, and move any explanatory messaging to the CLI layer (e.g. configCommand) where stdout output is expected.
Suggested implementation:
function get(key) {
// PROJECT_ROOT is always the current working directory
if (key === 'PROJECT_ROOT') {
// PROJECT_ROOT is always derived from the current working directory
return process.cwd();
}
return config.get(key);You’ll also want to adjust the set implementation in this same file to avoid console side effects:
- Find the
setfunction that currently looks like:
function set(key, value) {
// PROJECT_ROOT cannot be saved - always uses cwd
if (key === 'PROJECT_ROOT') {
console.log('Note: PROJECT_ROOT is always the current working directory and cannot be changed.');
return;
}
// ... probably existing logic here ...
}- Change it to silently ignore attempts to set
PROJECT_ROOT:
function set(key, value) {
// PROJECT_ROOT cannot be saved - always uses cwd
if (key === 'PROJECT_ROOT') {
// Silently ignore attempts to set PROJECT_ROOT from library code
return;
}
return config.set(key, value);
}This removes the stdout side effect from the library API while still preventing PROJECT_ROOT from being set. If your existing set logic differs, keep the existing behavior for non-PROJECT_ROOT keys and only replace the console.log branch with the early return.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces significant refactoring and feature enhancements to the vNext workflow CLI. Key changes include updating the README.md to detail a new vnext.config.json file for project configuration, revising command descriptions (check, sync, update, reset, csx, config), and providing a comprehensive command comparison table. The CLI's internal logic has been updated to dynamically read configuration from vnext.config.json, removing the need to manually set PROJECT_ROOT and API_DOMAIN via wf config. The package.json and package-lock.json files reflect a package name change from @vnext/workflow-cli to @burgan-tech/vnext-workflow-cli. Core functionalities in src/commands and src/lib have been refactored to use a new publishComponent API endpoint, improve error handling, and standardize console output with a new LOG helper. The csx processing now supports both Base64 and native text encoding for CSX files within JSONs, and component discovery is now strictly based on paths defined in vnext.config.json. Review comments highlighted a typo in the wf reset menu example, suggesting ALL instead of TUMU, and requested moving require statements to the top of src/lib/csx.js and src/lib/workflow.js for better code clarity.
| schemas (Schemas/) | ||
| ────────────── | ||
| 🔴 ALL (All folders) | ||
| TUMU (All folders) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const { exec } = require('child_process'); | ||
| const util = require('util'); | ||
| const execPromise = util.promisify(exec); | ||
| const fsSync = require('fs'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const util = require('util'); | ||
| const execPromise = util.promisify(exec); | ||
| const fs = require('fs'); | ||
| const fsSync = require('fs'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Major update to CLI documentation in README.md, including detailed usage, command explanations, and vnext.config.json support. Adds new src/lib/vnextConfig.js for reading project config, improves logging and user feedback in check, csx, reset, and sync commands, and updates package metadata. Enhances component discovery, error handling, and summary reporting for all major commands.
Summary by Sourcery
Document vNext CLI configuration via vnext.config.json, tighten component discovery to config-defined paths, and enhance core commands and logging around CSX processing and component publishing.
New Features:
Enhancements:
Build:
Documentation: