Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/@aws-cdk/toolkit-lib/lib/api/notices/notices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ export class Notices {
* Refresh the list of notices this instance is aware of.
*
* This method throws an error if the data source fails to fetch notices.
* When using, consider if execution should halt immdiately or if catching the rror and continuing is more appropriate
* When using, consider if execution should halt immdiately or if catching the error and continuing is more appropriate
*
* @throws on failure to refresh the data source
*/
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/cli/cli-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export async function makeConfig(): Promise<CliConfig> {
'role-arn': { type: 'string', alias: 'r', desc: 'ARN of Role to use when invoking CloudFormation', default: undefined, requiresArg: true },
'staging': { type: 'boolean', desc: 'Copy assets to the output directory (use --no-staging to disable the copy of assets which allows local debugging via the SAM CLI to reference the original source files)', default: true },
'output': { type: 'string', alias: 'o', desc: 'Emits the synthesized cloud assembly into a directory (default: cdk.out)', requiresArg: true },
'notices': { type: 'boolean', desc: 'Show relevant notices', default: YARGS_HELPERS.shouldDisplayNotices() },
'notices': { type: 'boolean', desc: 'Show relevant notices' },
'no-color': { type: 'boolean', desc: 'Removes colors and other style from console output', default: false },
'ci': { type: 'boolean', desc: 'Force CI detection. If CI=true then logs will be sent to stdout instead of stderr', default: YARGS_HELPERS.isCI() },
'unstable': { type: 'array', desc: 'Opt in to unstable features. The flag indicates that the scope and API of a feature might still change. Otherwise the feature is generally production ready and fully supported. Can be specified multiple times.', default: [] },
Expand Down
33 changes: 31 additions & 2 deletions packages/aws-cdk/lib/cli/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { ChangeSetDeployment, DeploymentMethod, DirectDeployment } from '@a
import { ToolkitError, Toolkit } from '@aws-cdk/toolkit-lib';
import * as chalk from 'chalk';
import { CdkToolkit, AssetBuildTime } from './cdk-toolkit';
import { ciSystemIsStdErrSafe } from './ci-systems';
import { displayVersionMessage } from './display-version';
import type { IoMessageLevel } from './io-host';
import { CliIoHost } from './io-host';
Expand Down Expand Up @@ -33,6 +34,7 @@ import type { StackSelector, Synthesizer } from '../cxapp';
import { ProxyAgentProvider } from './proxy-agent';
import { cdkCliErrorName } from './telemetry/error';
import type { ErrorDetails } from './telemetry/schema';
import { isCI } from './util/ci';
import { isDeveloperBuildVersion, versionWithBuild, versionNumber } from './version';

if (!process.stdout.isTTY) {
Expand Down Expand Up @@ -109,7 +111,34 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
await ioHost.asIoHelper().defaults.trace(`Telemetry instantiation failed: ${e.message}`);
}

const shouldDisplayNotices = configuration.settings.get(['notices']);
/**
* The default value for displaying (and refreshing) notices on all commands.
*
* If the user didn't supply either `--notices` or `--no-notices`, we do
* autodetection. The autodetection currently is: do write notices if we are
* not on CI, or are on a CI system where we know that writing to stderr is
* safe. We fail "closed"; that is, we decide to NOT print for unknown CI
* systems, even though technically we maybe could.
*/
const isSafeToWriteNotices = !isCI() || Boolean(ciSystemIsStdErrSafe());

// Determine if notices should be displayed based on CLI args and configuration
let shouldDisplayNotices: boolean;
if (argv.notices !== undefined) {
// CLI argument takes precedence
shouldDisplayNotices = argv.notices;
} else {
// Fall back to configuration file setting, then autodetection
const configNotices = configuration.settings.get(['notices']);
if (configNotices !== undefined) {
// Consider string "false" to be falsy in this context
shouldDisplayNotices = configNotices !== "false" && Boolean(configNotices);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Handle "false" as falsy.

Handle "", 0, null, and undefined as falsy.

} else {
// Default autodetection behavior
shouldDisplayNotices = isSafeToWriteNotices;
}
}

// Notices either go to stderr, or nowhere
ioHost.noticesDestination = shouldDisplayNotices ? 'stderr' : 'drop';
const notices = Notices.create({
Expand Down Expand Up @@ -193,7 +222,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
includeAcknowledged: !argv.unacknowledged,
showTotal: argv.unacknowledged,
});
} else if (cmd !== 'version') {
} else if (shouldDisplayNotices && cmd !== 'version') {
await notices.display();
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/cli/parse-command-line-arguments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export function parseCommandLineArguments(args: Array<string>): any {
requiresArg: true,
})
.option('notices', {
default: helpers.shouldDisplayNotices(),
default: undefined,
type: 'boolean',
desc: 'Show relevant notices',
})
Expand Down
15 changes: 0 additions & 15 deletions packages/aws-cdk/lib/cli/util/yargs-helpers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { ciSystemIsStdErrSafe } from '../ci-systems';
import { isCI } from '../util/ci';
import { versionWithBuild } from '../version';

export { isCI } from '../util/ci';
Expand Down Expand Up @@ -48,16 +46,3 @@ export function browserForPlatform(): string {
return 'xdg-open %u';
}
}

/**
* The default value for displaying (and refreshing) notices on all commands.
*
* If the user didn't supply either `--notices` or `--no-notices`, we do
* autodetection. The autodetection currently is: do write notices if we are
* not on CI, or are on a CI system where we know that writing to stderr is
* safe. We fail "closed"; that is, we decide to NOT print for unknown CI
* systems, even though technically we maybe could.
*/
export function shouldDisplayNotices(): boolean {
return !isCI() || Boolean(ciSystemIsStdErrSafe());
}
Comment on lines -61 to -63
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this logic to cli code.

2 changes: 1 addition & 1 deletion packages/aws-cdk/test/cli/cli-arguments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('yargs', () => {
lookups: true,
trace: undefined,
unstable: [],
notices: expect.any(Boolean),
notices: undefined,
output: undefined,
},
deploy: {
Expand Down
Loading
Loading