feat(update-offer): add action to update existing SPCTL offers with new configuration#19
feat(update-offer): add action to update existing SPCTL offers with new configuration#19
Conversation
|
""" WalkthroughA new composite GitHub Action named "Update offer" has been introduced. It takes inputs for an SPCTL config file, offer ID, and a new configuration file, validates the configuration, and updates an existing SPCTL offer using the provided files and parameters. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHub Actions Runner
participant SPCTL CLI
User->>GitHub Actions Runner: Trigger "Update offer" action with inputs
GitHub Actions Runner->>GitHub Actions Runner: Check SPCTL environment
GitHub Actions Runner->>GitHub Actions Runner: Validate configuration file (existence, JSON)
alt Validation fails
GitHub Actions Runner->>User: Exit with error
else Validation succeeds
GitHub Actions Runner->>SPCTL CLI: Run `spctl offers update` with offer ID and config files
SPCTL CLI-->>GitHub Actions Runner: Update result
GitHub Actions Runner->>User: Output success message
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
actions/update-offer/action.yml (1)
44-52: Fix the SPCTL update invocation.The command
./spctl offers update value ${{ inputs.offer_id }}incorrectly injects the literalvalueand invokes a local binary. It should call the installedspctldirectly and omitvalue.Apply this diff:
- ./spctl offers update value ${{ inputs.offer_id }} --configuration ${{ inputs.configuration_file_path }} --config ${{ inputs.spctl_config_file }} + spctl offers update ${{ inputs.offer_id }} \ + --configuration "${{ inputs.configuration_file_path }}" \ + --config "${{ inputs.spctl_config_file }}"
🧹 Nitpick comments (4)
actions/update-offer/action.yml (4)
1-3: Ensure consistent quoting style.You use single quotes for
namebut double quotes fordescription. For better readability and consistency, standardize on one quoting style (e.g., double quotes) throughout the YAML.
5-13: Harmonize input naming and validate SPCTL config.
- The inputs
spctl_config_filevsconfiguration_file_pathcould be unified (e.g., both ending in_fileor_path) for consistency.- Consider adding a validation step for the
spctl_config_file(analogous to the configuration file) to catch missing or invalid config early.
23-43: Scope validation is clear—but consider stricter shell settings.The file existence and JSON checks are well-structured. To catch unbound variables or pipeline errors, you might add at the top of the script:
set -euxo pipefailThis ensures the step fails fast on undefined variables or pipeline failures.
15-52: Consider exposing outputs from this composite action.It may be useful to define outputs (e.g., the
offer_idor a success flag) so downstream steps can reference the updated offer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
actions/update-offer/action.yml(1 hunks)
🔇 Additional comments (1)
actions/update-offer/action.yml (1)
18-22: Check SPCTL action usage is correct.The existing
check-spctl@v1step looks good and properly passes thespctl_config_fileinput.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
actions/update-offer/index.js (1)
16-21: Enhance JSON validation with structure checking.While basic JSON validation is good, consider validating the expected structure/schema of the configuration file to catch configuration errors early.
try { - JSON.parse(fs.readFileSync(configFilePath, 'utf8')); + const config = JSON.parse(fs.readFileSync(configFilePath, 'utf8')); + // Add basic structure validation + if (!config || typeof config !== 'object') { + throw new Error('Configuration must be a valid JSON object'); + } core.info('✅ File content is valid JSON.'); } catch (error) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
actions/update-offer/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
.gitignore(1 hunks)actions/update-offer/action.yml(1 hunks)actions/update-offer/index.js(1 hunks)actions/update-offer/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- actions/update-offer/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- actions/update-offer/action.yml
🔇 Additional comments (2)
actions/update-offer/index.js (2)
1-3: LGTM! Clean imports setup.The required dependencies are properly imported for GitHub Actions functionality.
35-37: LGTM! Proper error handling implementation.The error handling correctly uses
core.setFailed()to report failures to the GitHub Actions runtime.
…ge.json, add README.md (SP-6620)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
actions/update-offer/README.md (2)
57-60: Ensure consistent casing for thespctlutility
The step title uses "SPCTL" in all caps, while elsewhere the utility is referenced asspctl. Use backticks and a consistent lowercase form (e.g.,spctl) for clarity.
75-81: Refine repeated sentence openings
Multiple consecutive sentences start with "This". Consider merging or rephrasing to improve readability. For example:Building the action is essential after code changes: use `@vercel/ncc` to compile your code and dependencies into the `dist` directory, since `node_modules` isn’t checked in.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
actions/update-offer/dist/index.jsis excluded by!**/dist/**actions/update-offer/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
actions/update-offer/README.md(1 hunks)actions/update-offer/action.yml(1 hunks)actions/update-offer/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- actions/update-offer/action.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- actions/update-offer/package.json
🧰 Additional context used
🪛 LanguageTool
actions/update-offer/README.md
[style] ~77-~77: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e file located in the dist directory. This is necessary because the node_modules...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (1)
actions/update-offer/README.md (1)
1-6: Clear and concise action overview
The introduction accurately summarizes the purpose and usage of the action.
…022 and add minify (SP-6620)
@marchuk-vlad @Idris0v @serega-k |
I have nothing against using a Node.js action, but in this case, I agree with @marchuk-vlad |
3 participants
Summary by CodeRabbit