Skip to content

Commit

Permalink
fix(pacmak): don't automatically translate examples without asking (#…
Browse files Browse the repository at this point in the history
…3219)

Given the current state of example tooling, the existing default
was a bad choice.

It forces everyone who is publishing jsii packages everywhere to
spend CPU cycles to do example parsing and translation which is
bound to fail anyway (as the fixtures and compilation directory
will probably not be set up for success, and translation
must proceed in a single-threaded fashion).

The new default is to use translations if they exist, and otherwise
don't do translations.

* Builds that care (i.e., mostly the CDK build), will be using `rosetta
  extract` already to be efficient and correct about compilation, and
  the tablet files will be picked up as usual.
* Builds that don't care will get more efficient.

BREAKING CHANGES: Pacmak no longer tries to translate examples
by default: this feature is now opt-in.



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
rix0rrr authored Dec 3, 2021
1 parent 6ac9fef commit 937f8c3
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 23 deletions.
22 changes: 17 additions & 5 deletions packages/jsii-pacmak/bin/jsii-pacmak.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ import { VERSION_DESC } from '../lib/version';
.option('rosetta-translate-live', {
type: 'boolean',
desc: "Translate code samples on-the-fly if they can't be found in the samples tablet (deprecated)",
default: true,
default: undefined,
})
.option('rosetta-unknown-snippets', {
type: 'string',
Expand Down Expand Up @@ -135,6 +135,21 @@ import { VERSION_DESC } from '../lib/version';
// Default to 4 threads in case of concurrency, good enough for most situations
debug('command line arguments:', argv);

if (
argv['rosetta-translate-live'] !== undefined &&
argv['rosetta-unknown-snippets'] !== undefined
) {
throw new Error(
'Prefer using --rosetta-unknown-snippets over --rosetta-translate-live',
);
}

const rosettaUnknownSnippets =
(argv['rosetta-unknown-snippets'] as UnknownSnippetMode | undefined) ??
(argv['rosetta-translate-live']
? UnknownSnippetMode.TRANSLATE
: UnknownSnippetMode.VERBATIM);

return pacmak({
argv,
clean: argv.clean,
Expand All @@ -147,10 +162,7 @@ import { VERSION_DESC } from '../lib/version';
outputDirectory: argv.outdir,
parallel: argv.parallel,
recurse: argv.recurse,
rosettaLiveConversion: argv['rosetta-translate-live'],
rosettaUnknownSnippets: argv['rosetta-unknown-snippets'] as
| UnknownSnippetMode
| undefined,
rosettaUnknownSnippets,
rosettaTablet: argv['rosetta-tablet'],
targets: argv.targets?.map((target) => target as TargetName),
updateNpmIgnoreFiles: argv.npmignore,
Expand Down
20 changes: 2 additions & 18 deletions packages/jsii-pacmak/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,14 @@ export async function pacmak({
outputDirectory,
parallel = true,
recurse = false,
rosettaLiveConversion,
rosettaTablet,
targets = Object.values(TargetName),
timers = new Timers(),
rosettaUnknownSnippets = undefined,
updateNpmIgnoreFiles = false,
validateAssemblies = false,
}: PacmakOptions): Promise<void> {
const unknownSnippets =
rosettaUnknownSnippets ??
(rosettaLiveConversion
? UnknownSnippetMode.TRANSLATE
: UnknownSnippetMode.VERBATIM);

const rosetta = new Rosetta({ unknownSnippets });
const rosetta = new Rosetta({ unknownSnippets: rosettaUnknownSnippets });
if (rosettaTablet) {
await rosetta.loadTabletFromFile(rosettaTablet);
}
Expand Down Expand Up @@ -236,19 +229,10 @@ export interface PacmakOptions {
*/
readonly recurse?: boolean;

/**
* Whether `jsii-rosetta` conversion should be performed in-band for examples found in documentation which are not
* already translated in the `rosettaTablet` file.
*
* @default false
* @deprecated Use `rosettaUnknownSnippets` instead.
*/
readonly rosettaLiveConversion?: boolean;

/**
* How rosetta should treat snippets that cannot be loaded from a translation tablet.
*
* @default - falls back to the default of `rosettaLiveConversion`.
* @default UnknownSnippetMode.VERBATIM
*/
readonly rosettaUnknownSnippets?: UnknownSnippetMode;

Expand Down

0 comments on commit 937f8c3

Please sign in to comment.