Repository navigation
fix(utils): insert no config a target does not declare, and reject release-suppressing markers - #277
Merged
Merged
Conversation
…lease-suppressing markers
Two fixes, both about something being added that should not be.
Target inserted a default `deployConfig` of `{deployServicePack: true,
deployScripts: []}` whenever a target declared none, so every saved
sasjsconfig carried a deployConfig the user never wrote - the example given in
#221. That insertion is gone.
`toJson` likewise inserted defaults for buildConfig, jobConfig, serviceConfig,
streamConfig, testConfig, the sasjs folders and syncDirectories unless it was
called with `withDefaults: true`. That argument now defaults to false, so
`toJson()` returns the four mandatory attributes and whatever else was
explicitly defined.
Closes #221.
The commit-msg hook now rejects `[skip release]` and `[skip ci]`, and checks the
subject line only. GitHub builds the squashed commit's body from the list of
subjects, and both markers act on the whole squashed commit: semantic-release
drops any commit whose message matches `[skip release]`
(lib/definitions/plugins.js), and `[skip ci]` makes GitHub skip the
push-triggered workflow. Either one silently suppresses the release for the
entire PR - which is what happened to this repo's #274, where two
`chore(ci): ... [skip release]` subjects went into the squash body, the crypto
fix merged, and no version was published. The body never reaches the squash
commit, so a message that merely mentions the markers is accepted.
Verification:
- npx tsc --noEmit: clean
- npx jest: 39 suites, 440 tests pass
- npm run lint: clean
- the hook against 8 subject shapes, including unscoped and breaking types
Contributor
Author
|
The CLI half of #221 is now open as sasjs/cli#1480.
Note the ordering: the CLI change passes explicit values, so it is safe now, but it only takes full effect once that repo's |
allanbowe
pushed a commit
that referenced
this pull request
Oct 5, 2026
* fix(ci): make the release dispatchable The merge of #277 produced no workflow run at all - no run, no check run - so nothing was published, and there was no way to trigger it by hand: this workflow triggers on `push` to main only, and that push produced nothing. `workflow_dispatch` makes the release reachable from the Actions UI or the API when the push trigger does not fire. It changes nothing about what a release does - semantic-release computes from the commits since the last tag either way. This is a workaround rather than the fix. The underlying problem is that this repo has stopped scheduling workflow runs: its most recent run is from 19:31, while cli, vscode-extension, test-framework and sas-language all triggered normally at the same moments. That needs someone with admin access to diagnose. What this does is ensure a missed release can be recovered instead of being stranded. Verification: the workflow file parses as YAML, and the only change is the trigger block. * test(target): cover the toJson defaults that the escape hatch inserts Flipping `toJson`'s default to false left the eight `else if (withDefaults)` branches uncovered, because no spec passed `true` any more. This repo gates on coverage, so CI failed on it: Jest: Uncovered count for statements (16) exceeds global threshold (10) All 440 tests passed - it was the gate, not a test. The uncovered lines were exactly 341, 347, 353, 359, 369, 379, 389 and 400, one per injected default. The new case asserts the escape hatch still does what it says: the four mandatory attributes plus the eight inserted defaults, which is what a caller passing `true` - and the CLI's `saveWithDefaultValues` flag - relies on. Verification: npx jest --config=jest.config.js --coverage - 39 suites, 441 tests pass, no threshold error; npx tsc --noEmit clean; npm run lint clean. --------- Co-authored-by: sasjs-dev[bot] <sasjs-dev[bot]@users.noreply.github.com>
sasjs-gha Bot
pushed a commit
that referenced
this pull request
Oct 5, 2026
## [3.7.1](v3.7.0...v3.7.1) (2026-10-05) ### Bug Fixes * **ci:** make the release dispatchable ([#278](#278)) ([bf2f677](bf2f677)), closes [#277](#277) * **utils:** drop the Node crypto import, and tighten what the package ships ([#274](#274)) ([0f79733](0f79733)) * **utils:** insert no config a target does not declare, and reject release-suppressing markers ([#277](#277)) ([58baebb](58baebb)), closes [#274](#274)
|
🎉 This PR is included in version 3.7.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
sasjs-dev Bot
added a commit
to sasjs/cli
that referenced
this pull request
Oct 6, 2026
Two review findings. - Both flipped functions now carry a JSDoc stating the new contract: omitting `saveWithDefaultValues` persists a minimal target - the mandatory attributes only - and passing true writes the full set of injected defaults, including a deployConfig the caller never declared. The behaviour inverted, so the signature should say so rather than leave the next caller to infer it. - The pin moves to @sasjs/utils 3.7.1, the release carrying the constructor fix from sasjs/utils#277. Without it this change is half-effective: 3.6.5's Target constructor inserts a default deployConfig for any target that declares none, so toJson(false) still emits it after the flip. sasjs/adapter#919 has already moved to 3.7.1; this follows it. Verified: `tsc --noEmit` clean, config.spec 39 tests passing, and package.json and the lockfile root both at 3.7.1.
sasjs-dev Bot
added a commit
to sasjs/cli
that referenced
this pull request
Oct 6, 2026
The @sasjs/utils 3.7.1 bump in this PR exposed a downstream dependency. The
Target constructor used to inject
`{ deployServicePack: true, deployScripts: [] }` into any target that declared
no deployConfig. sasjs/utils#277 removed that injection, so after the bump a
target without a deployConfig has none at all - and deploy() then refuses:
Deployment failed.
Please either enable the 'deployServicePack' option or add deployment
script paths to 'deployScripts' in your target's 'deployConfig'.
That is a user-facing regression: any existing sasjsconfig that omits
deployConfig deployed before this and would stop deploying once the pin moves.
Fixed where it belongs - in the CLI. An absent deployConfig now means the
historical default, deploy the service pack. An explicit
`deployServicePack: false` is still honoured, and the guard that throws uses the
same resolved value rather than re-reading the raw config.
The utils change was about not writing defaults into the user's saved config. It
was never meant to change runtime behaviour, and this restores that.
Verified: tsc clean; deploy specs 28 tests passing; config and add specs 73
passing. The job that caught it - the SAS9 mocks deploy against a real
sasjs/server - is the end-to-end proof and will re-run on this head.
sasjs-dev Bot
added a commit
to sasjs/cli
that referenced
this pull request
Oct 14, 2026
`saveToLocalConfig` and `saveToGlobalConfig` defaulted `saveWithDefaultValues` to true, so any caller that omitted the argument wrote every non-mandatory attribute into the saved sasjsconfig. The token-refresh handler does exactly that - it passes only the target and isDefault - so every token refresh rewrote the global target with a full set of defaults, including a `deployConfig` the user never wrote. That is the other half of #221. The constructor and `toJson` halves are in sasjs/utils#277; this repo passes explicit values on the `sasjs add` path, so that path is unchanged, but the refresh path was writing full configs on its own. The specs that relied on the default are unaffected: they assert that the target is present and whether it is the default, not the JSON shape. Verification: - npx jest src/utils/spec/config.spec.ts: 39 pass - npx jest src/commands/add: 3 suites, 34 tests pass - npx tsc -p tsconfig.json --noEmit: clean - npm run lint: clean Takes full effect once this repo's `@sasjs/utils` pin moves to the release carrying the constructor fix (utils 3.7.1, from sasjs/utils#277) - until then the constructor still inserts `deployConfig` before `toJson` is reached. (cherry picked from commit 7b5725f)
sasjs-dev Bot
added a commit
to sasjs/cli
that referenced
this pull request
Oct 14, 2026
Two review findings. - Both flipped functions now carry a JSDoc stating the new contract: omitting `saveWithDefaultValues` persists a minimal target - the mandatory attributes only - and passing true writes the full set of injected defaults, including a deployConfig the caller never declared. The behaviour inverted, so the signature should say so rather than leave the next caller to infer it. - The pin moves to @sasjs/utils 3.7.1, the release carrying the constructor fix from sasjs/utils#277. Without it this change is half-effective: 3.6.5's Target constructor inserts a default deployConfig for any target that declares none, so toJson(false) still emits it after the flip. sasjs/adapter#919 has already moved to 3.7.1; this follows it. Verified: `tsc --noEmit` clean, config.spec 39 tests passing, and package.json and the lockfile root both at 3.7.1. (cherry picked from commit c8560b7)
sasjs-dev Bot
added a commit
to sasjs/cli
that referenced
this pull request
Oct 14, 2026
The @sasjs/utils 3.7.1 bump in this PR exposed a downstream dependency. The
Target constructor used to inject
`{ deployServicePack: true, deployScripts: [] }` into any target that declared
no deployConfig. sasjs/utils#277 removed that injection, so after the bump a
target without a deployConfig has none at all - and deploy() then refuses:
Deployment failed.
Please either enable the 'deployServicePack' option or add deployment
script paths to 'deployScripts' in your target's 'deployConfig'.
That is a user-facing regression: any existing sasjsconfig that omits
deployConfig deployed before this and would stop deploying once the pin moves.
Fixed where it belongs - in the CLI. An absent deployConfig now means the
historical default, deploy the service pack. An explicit
`deployServicePack: false` is still honoured, and the guard that throws uses the
same resolved value rather than re-reading the raw config.
The utils change was about not writing defaults into the user's saved config. It
was never meant to change runtime behaviour, and this restores that.
Pinned by a new case in deploy.spec.ts, beside the existing SAS Viya
service-pack case: a target that declares no deployConfig, asserting the
service-pack deploy path runs. The rest of the suite declares a deployConfig
everywhere, which is why the runtime regression reached CI before any spec
caught it.
Verified: tsc clean; deploy specs 29 tests passing; config and add specs 73
passing. The job that caught it - the SAS9 mocks deploy against a real
sasjs/server - is the end-to-end proof and will re-run on this head.
allanbowe
pushed a commit
to sasjs/cli
that referenced
this pull request
Oct 14, 2026
…22 (#1482) * fix(package): require the oldest supported LTS (node >= 22) The declared node floor permits end-of-life releases - node 14 and 16 reached EOL in 2023, 18 on 2025-04-30, and 20 on 2026-04-30. The oldest line still receiving security fixes is 22, which is also the version CI runs. Raise engines.node to >=22 so the declaration matches the runtimes this package is tested and supported on. Verification: the only package.json change is the engines.node value, and CI runs on node 22. (cherry picked from commit c6e342e) * fix(package): require the oldest supported LTS (node >= 22) The declared node floor permits end-of-life releases - node 14 and 16 reached EOL in 2023, 18 on 2025-04-30, and 20 on 2026-04-30. The oldest line still receiving security fixes is 22, which is also the version CI runs. Raise engines.node to >=22 so the declaration matches the runtimes this package is tested and supported on. Verification: the only package.json change is the engines.node value, and CI runs on node 22. BREAKING CHANGE: engines.node is now >= 22; node 18 and 20 users must stay on the previous release. (cherry picked from commit c6e342e) * fix(cli): declare axios, drop the dead version scripts, pin the release tooling Three findings from review, plus one of the same staleness class: - axios is imported by src/utils/utils.ts and src/commands/run/run.ts but was declared nowhere - it rode on the hoist from @sasjs/adapter's dependency. Declared directly, pinned to 1.20.0 (the version already resolved, so the tree does not move). - the `checkNodeVersion` and `nodeVersionMessage` npm scripts are referenced by no workflow and no doc, and the check embedded "^14.18.0 || >=16.10.0" with `m > 16` logic, so it would accept node 18/20 that engines now rejects. Dropped both; engines does the job. - src/utils/utils.ts's checkNodeVersion() refused only below node 12, and said so to the user. Raised to 22, matching engines, with its spec updated. - .github/workflows/npmpublish.yml ran `npx -p ... semantic-release` unpinned, resolving latest on every release with publish credentials. Pinned to the versions the other sasjs release workflows use. Verification: - npm ci: 698 packages - npx jest src/utils/spec/utils.spec.ts: 46 tests pass - npm run lint: clean - package-lock.json root now records axios and engines >=22 Note: `npm run test:mocked` bails on src/commands/version/spec/version.spec.ts in a sandbox, because printVersion() shells out to `npm list -g @sasjs/cli` and nothing is installed globally. Reproduced identically on the base commit with these changes stashed, so it is environmental, not this diff. (cherry picked from commit 71a639d) * fix(ci): install the release tooling from a locked tree The release toolchain now lives in `release/` with its own package.json and lockfile, installed by the publish workflow with `npm ci --prefix release`, rather than being resolved from npx at release time. Locked, so the resolved versions and their integrity hashes are pinned - the gap raised in review, where npx pinned only the top levels and fetched the transitive tree fresh. The `files` allowlist already keeps `release/` out of the tarball. Verification: every plugin in the release config resolves from release/node_modules. (cherry picked from commit 5242e0a) * chore(ci): audit the release toolchain in its own scope The toolchain in release/ carries twelve advisories that cannot be fixed from this repo - the ranges cover the newest published version - and that code runs with publish credentials. Moving it out of the package's dependencies kept it out of the package's `npm audit`, which is right; it also made it invisible, which is not. release/audit.mjs reports it separately and by name: the twelve are exempt with their reasons, anything else fails the build, and an exemption that stops appearing is reported so it can be deleted rather than left to rot. It uses --package-lock-only, so it needs no install and runs on every PR as well as in the release. Verification: node release/audit.mjs reports 12 exempt and exits 0. (cherry picked from commit f1c4506) * chore(ci): format the release audit script for the prettier gate sas-language runs `prettier --check .`, which covers release/ - it is the only one of the five repos that checks that directory - and it rejected the new script. Formatted with that config (printWidth 100). The file is kept byte-identical in all five repos, so the same content goes to the four that do not check it. Otherwise the copies drift and a later fix lands in some of them only. Verification: `prettier --check .` clean, `node release/audit.mjs` still reports 12 exempt and exits 0, in all five. (cherry picked from commit ce73dd9) * fix(ci): give the release checkout the full history The publish workflow's checkout is shallow (depth 1), and semantic-release needs the full history and tags to compute the version. It fetches --unshallow itself, which works on a public repo but is a wasted round-trip on every release and fails outright if the fetch is blocked. The sibling release workflows (adapter, utils, sas-language) all set fetch-depth: 0 for this reason. (cherry picked from commit 331c4fd) * chore(utils): drop the unreachable node-version check `checkNodeVersion` in src/utils/utils.ts has no caller: src/index.ts does not call it, it is not re-exported from cli.ts or index.ts, and nothing else in the repo references it. It was already unreachable before this PR; this PR edited it (the floor and the message), which is the moment to settle it either way. Dropped rather than wired into the entrypoint, because as written it would fail silently there: it reports through `process.logger?.error`, and the entrypoint has not set up a logger yet, so the optional chain short-circuits and the process exits 1 with no message at all. `engines: ">=22"` is the enforcement, and npm surfaces it on install. The spec block covering the function goes with it. Verification: tsc -p tsconfig.json clean, prettier clean, src/utils/spec 44 tests pass (was 46). (cherry picked from commit b82f4fd) * fix(ci): name the postcss-selector-parser advisory in the toolchain audit The audit caught a new advisory, which is what it is for: GHSA-rj75-hqrm-r3gf, moderate, quadratic complexity in flat selector parsing. It cannot be fixed from here. The vulnerable copy is npm's own bundled dependency - release/node_modules/npm/node_modules/postcss-selector-parser - so no resolver change reaches it, and 11.21.0 is the last npm 11.x while @semantic-release/npm requires ^11. The eventual fix is that plugin moving to npm 12. It is also unreachable in this usage: the parser reads this repo's own release config, not the commits being matched. Named with that reasoning rather than silenced, so the audit still fails on anything new. 13 exempt, exit 0. (cherry picked from commit 3e02d7a) * fix(ci): name the postcss-selector-parser advisory in the toolchain audit The audit caught a new advisory, which is what it is for: GHSA-rj75-hqrm-r3gf, moderate, quadratic complexity in flat selector parsing. It cannot be fixed from here. The vulnerable copy is npm's own bundled dependency - release/node_modules/npm/node_modules/postcss-selector-parser - so no resolver change reaches it, and 11.21.0 is the last npm 11.x while @semantic-release/npm requires ^11. The eventual fix is that plugin moving to npm 12. It is also unreachable in this usage: the parser reads this repo's own release config, not the commits being matched. Named with that reasoning rather than silenced, so the audit still fails on anything new. 13 exempt, exit 0. (cherry picked from commit 70c4ace) * chore(deps): drop four declarations nothing references Surfaced by the depcheck pass and carried through several reviews. Verified by full-tree grep before removing: - jwt-decode 3.1.2 and lodash.groupby 4.6.0 were runtime dependencies, but nothing in src, docs or scripts imports jwtDecode or groupBy. lodash.uniqby beside them is used; these two were not, so every @sasjs/cli consumer installed them for nothing. - tslib 2.8.1 was a devDependency with no reference anywhere, and tsconfig sets no importHelpers to justify it. - @types/lodash.groupby 4.6.9 loses its reason with lodash.groupby gone. Verified after removal: `tsc --noEmit` clean, and the utils and config specs run 83 tests, all passing. (cherry picked from commit ef17b9a) * fix(config): save a minimal target unless defaults are asked for `saveToLocalConfig` and `saveToGlobalConfig` defaulted `saveWithDefaultValues` to true, so any caller that omitted the argument wrote every non-mandatory attribute into the saved sasjsconfig. The token-refresh handler does exactly that - it passes only the target and isDefault - so every token refresh rewrote the global target with a full set of defaults, including a `deployConfig` the user never wrote. That is the other half of #221. The constructor and `toJson` halves are in sasjs/utils#277; this repo passes explicit values on the `sasjs add` path, so that path is unchanged, but the refresh path was writing full configs on its own. The specs that relied on the default are unaffected: they assert that the target is present and whether it is the default, not the JSON shape. Verification: - npx jest src/utils/spec/config.spec.ts: 39 pass - npx jest src/commands/add: 3 suites, 34 tests pass - npx tsc -p tsconfig.json --noEmit: clean - npm run lint: clean Takes full effect once this repo's `@sasjs/utils` pin moves to the release carrying the constructor fix (utils 3.7.1, from sasjs/utils#277) - until then the constructor still inserts `deployConfig` before `toJson` is reached. (cherry picked from commit 7b5725f) * fix(config): document the minimal-save contract and move the utils pin Two review findings. - Both flipped functions now carry a JSDoc stating the new contract: omitting `saveWithDefaultValues` persists a minimal target - the mandatory attributes only - and passing true writes the full set of injected defaults, including a deployConfig the caller never declared. The behaviour inverted, so the signature should say so rather than leave the next caller to infer it. - The pin moves to @sasjs/utils 3.7.1, the release carrying the constructor fix from sasjs/utils#277. Without it this change is half-effective: 3.6.5's Target constructor inserts a default deployConfig for any target that declares none, so toJson(false) still emits it after the flip. sasjs/adapter#919 has already moved to 3.7.1; this follows it. Verified: `tsc --noEmit` clean, config.spec 39 tests passing, and package.json and the lockfile root both at 3.7.1. (cherry picked from commit c8560b7) * fix(deploy): treat a missing deployConfig as deployServicePack The @sasjs/utils 3.7.1 bump in this PR exposed a downstream dependency. The Target constructor used to inject `{ deployServicePack: true, deployScripts: [] }` into any target that declared no deployConfig. sasjs/utils#277 removed that injection, so after the bump a target without a deployConfig has none at all - and deploy() then refuses: Deployment failed. Please either enable the 'deployServicePack' option or add deployment script paths to 'deployScripts' in your target's 'deployConfig'. That is a user-facing regression: any existing sasjsconfig that omits deployConfig deployed before this and would stop deploying once the pin moves. Fixed where it belongs - in the CLI. An absent deployConfig now means the historical default, deploy the service pack. An explicit `deployServicePack: false` is still honoured, and the guard that throws uses the same resolved value rather than re-reading the raw config. The utils change was about not writing defaults into the user's saved config. It was never meant to change runtime behaviour, and this restores that. Pinned by a new case in deploy.spec.ts, beside the existing SAS Viya service-pack case: a target that declares no deployConfig, asserting the service-pack deploy path runs. The rest of the suite declares a deployConfig everywhere, which is why the runtime regression reached CI before any spec caught it. Verified: tsc clean; deploy specs 29 tests passing; config and add specs 73 passing. The job that caught it - the SAS9 mocks deploy against a real sasjs/server - is the end-to-end proof and will re-run on this head. * fix(deps): bump adapter, core, utils and lint to their released versions @sasjs/adapter 4.19.2 -> 4.19.4 @sasjs/core 5.3.0 -> 5.3.2 @sasjs/lint 4.1.2 -> 4.1.3 @sasjs/utils 3.7.1 -> 3.7.2 adapter 4.19.4 carries the Socket alert cleanup (#915 + #917): tough-cookie 5.1.2 drops psl, punycode, url-parse, querystringify and requires-port from the prod tree, and the bundled @sasjs/utils drops cli-table, colors, lodash.isequal and lodash.isfunction. It also declares engines.node >= 18, the floor its own chain requires. core 5.3.2 is the mf_getvalue edge-case fix (#459), on top of 5.3.1's ms_runstp / ms_triggerstp / ms_testservice debug= parameter fix (#458). lint 4.1.3 requires node >= 22 and syncs its own lockfile engines, so it agrees with the engines floor this PR declares for the CLI. It still pins @sasjs/utils 3.6.4, so a nested utils copy stays in the tree under lint - the same exact-pin chain as adapter, and it converges when lint bumps its own pin. utils 3.7.2 is a CI-only change (the release-marker filter check, #279); no runtime difference from 3.7.1. handlebars 4.7.9 -> 4.7.10 in the release toolchain lockfile. Three advisories were published against 4.7.9 (GHSA-xw65-4hp5-5hc7, GHSA-8r5x-fm3f-whwj, GHSA-p8wg-vrv2-v86f), all of which 4.7.10 is the first patched version for. The release audit goes back to exit 0 with its 13 documented exemptions. The bump lands in release/package-lock.json only, which is where the toolchain resolves. Verified on this tree: build exit 0, 0 TS errors jest (mocked) 87 suites / 667 tests, 2 suites failing on the environment alone - docs (needs doxygen) and version (needs a global @sasjs/cli install); 21 tests, all in those two coverage 74.52 / 62.78 / 75.21 / 75.1, above the required 73 / 60.55 / 73.54 / 74 npm audit --omit=dev 0 vulnerabilities node release/audit.mjs 13 exempt, exit 0 * fix(test): assert the persisted shape of a minimal target save Adds the coverage the minimal-save flip was missing: one assertion each on the persisted JSON for saveToGlobalConfig and saveToLocalConfig, so the minimal-vs-defaults distinction has a spec pinning it. The minimal case constructs a target that declares no deployConfig, like the token-refresh path whose behaviour this is for, and asserts both deployConfig and buildConfig are absent from the saved JSON. The defaults case asserts the injected deployConfig is present when saveWithDefaultValues is true. Also makes a stale release-toolchain exemption fail the build, matching the script's own stated rule that an exemption which outlives its advisory should be deleted, not left to rot. Verified: config spec 43 passing; node release/audit.mjs exit 0; tsc clean; prettier clean. * fix(test): rename the defaults spec and fail on stale release-toolchain exemptions Renames the saveToGlobalConfig defaults case to match its saveToLocalConfig twin: the test passes saveWithDefaultValues explicitly, so "is true" is the accurate description, not "is omitted" - which under the new default of false describes the opposite save. Makes a stale exemption (an EXEMPT entry whose advisory no longer appears in the audit report) exit 1 on stderr, matching the script's own stated rule that an exemption which outlives its advisory should be deleted, not left to rot. Both branches exercised: the clean tree exits 0 (13 exempt), a stubbed bogus exemption id exits 1 and names it. * fix(ci): report a failing advisory even when a stale exemption exists The stale-exemption gate fired before the failing gate and exited, so when both existed the run reported only the stale exemption and never mentioned the unexempted advisory. Both blocks now report, and a single exit at the end fails the build if either did. Order preserved: the unexempted advisory, which is the reason the toolchain is unsafe to publish, prints before the housekeeping note about deletable exemptions. All four branches exercised: clean tree exits 0 (13 exempt); a stale exemption alone exits 1 and names it; an unexempted advisory alone exits 1 and names it; both together exit 1 and report both. --------- Co-authored-by: sasjs-dev[bot] <sasjs-dev[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two fixes, both about something being added that should not be.
A target no longer gains a
deployConfigit did not declare.Target's constructor inserted{ deployServicePack: true, deployScripts: [] }whenever a target declared none, so every saved sasjsconfig carried adeployConfigthe user never wrote. That is the example #221 gives, and it is now gone - a target that declares nodeployConfighas none.toJsonno longer inserts defaults unless asked. It filled inbuildConfig,jobConfig,serviceConfig,streamConfig,testConfig,sasjsBuildFolder,sasjsResultsFolderandsyncDirectoriesunless called withwithDefaults: true. That argument now defaults to false, sotoJson()returns the four mandatory attributes -name,serverUrl,serverType,appLoc- plus whatever was explicitly defined.Closes #221.
The commit-msg hook rejects release-suppressing markers.
[skip release]and[skip ci]are now refused in a commit subject, and the check looks at the subject line only. GitHub builds the squashed commit's body from the list of commit subjects, and both markers act on the whole squashed commit rather than the one commit carrying them: semantic-release drops any commit whose message matches[skip release](lib/definitions/plugins.js), and[skip ci]makes GitHub skip the push-triggered workflow outright.This is not hypothetical - it is what happened to this repo's #274. Two
chore(ci): ... [skip release]subjects went into the squash body, the squashed commit was filtered out whole, the crypto fix merged, and no version was ever published. The body never reaches the squash commit, so a message that merely mentions the markers is accepted; semantic-release's ownchore(release): X [skip ci]commit is allowed through as well, since it is pushed straight to the release branch.Verification
npx tsc --noEmit: cleannpx jest: 39 suites, 440 tests passnpm run lint: clean!, both markers are rejected, and a bare non-conventional subject is rejectedNotes
Two spec expectations asserted the inserted defaults and now assert the values are absent. One of them was in a test already named "should convert to JSON with minimal attributes", which had been asserting the opposite of its own name.
While updating them I found a pre-existing gap, left alone here and recorded in the specs rather than papered over:
Targetguards every config assignment withif (json.<config>), so a config present only in the configuration passed to the constructor is dropped. The old tests did not catch this because the injected default happened to equal the fixture. Worth its own issue.The CLI side is unchanged:
saveToLocalConfigandsaveToGlobalConfigpasssaveWithDefaultValues = true, so a saved config still gains thetoJsondefaults. Flipping that default is the other half of #221 and belongs in sasjs/cli - a companion PR rather than this one.