feat: add new builders for metadata migration check #1732
Merged
Conversation
This file contains 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
cpourcel
force-pushed
the
feature/metadata-migration-check
branch
from
815ce00
to
3a39949
Compare
cpourcel
changed the title
cpourcel
force-pushed
the
feature/metadata-migration-check
branch
from
3a39949
to
7b5fd7d
Compare
kpanot
reviewed
Apr 30, 2024
packages/@o3r/extractors/src/core/comparator/metadata-comparator.interface.ts
Outdated
Show resolved
Hide resolved
packages/@o3r/extractors/src/core/comparator/metadata-comparator.interface.ts
Outdated
Show resolved
Hide resolved
packages/@o3r/extractors/src/core/comparator/metadata-comparison.helper.ts
Show resolved
Hide resolved
| * @param context Builder context (from another builder) | ||
| * @param comparator Comparator implementation, depends on the type of metadata to check | ||
| */ | ||
| export async function checkMetadataBuilder<T>(options: MigrationMetadataCheckBuilderOptions, context: BuilderContext, comparator: MetadataComparator<T>): Promise<BuilderOutput> { |
packages/@o3r/extractors/src/core/comparator/metadata-comparison.helper.ts
Outdated
Show resolved
Hide resolved
kpanot
reviewed
Apr 30, 2024
packages/@o3r/extractors/src/core/comparator/metadata-comparison.helper.ts
Outdated
Show resolved
Hide resolved
packages/@o3r/extractors/src/core/comparator/metadata-files.helper.ts
Outdated
Show resolved
Hide resolved
| * Read and parses a JSON file | ||
| * @param metadataPath Path of the file | ||
| */ | ||
| export function getLocalMetadataFile<T>(metadataPath: string): T { |
There was a problem hiding this comment.
Should add constraint on T (like JsonObject from type-fest for example)
packages/@o3r/extractors/src/core/comparator/metadata-files.helper.ts
Outdated
Show resolved
Hide resolved
...3r/extractors/src/core/comparator/package-managers-extractors/yarn2-file-extractor.helper.ts
Outdated
Show resolved
Hide resolved
...3r/extractors/src/core/comparator/package-managers-extractors/yarn2-file-extractor.helper.ts
Outdated
Show resolved
Hide resolved
...@o3r/localization/builders/metadata-check/helpers/localization-metadata-comparison.helper.ts
Show resolved
Hide resolved
packages/@o3r/styling/builders/metadata-check/helpers/styling-metadata-comparison.helper.ts
Outdated
Show resolved
Hide resolved
|
The following items are missing to the feature:
|
cpourcel
force-pushed
the
feature/metadata-migration-check
branch
from
7b5fd7d
to
105a0e8
Compare
pginoux-1A
reviewed
Apr 30, 2024
pginoux-1A
reviewed
Apr 30, 2024
pginoux-1A
reviewed
Apr 30, 2024
pginoux-1A
reviewed
Apr 30, 2024
pginoux-1A
reviewed
Apr 30, 2024
pginoux-1A
reviewed
Apr 30, 2024
pginoux-1A
reviewed
Apr 30, 2024
pginoux-1A
reviewed
Apr 30, 2024
packages/@o3r/extractors/src/core/comparator/metadata-comparison.helper.ts
Outdated
Show resolved
Hide resolved
pginoux-1A
reviewed
May 23, 2024
| }, | ||
| "packageManager": { | ||
| "type": "string", | ||
| "description": "Override of the package manager, otherwise it will be determined from the project." |
There was a problem hiding this comment.
From the angular.json or the process env ? Since you are calling the getPackageManager() function
pginoux-1A
reviewed
May 23, 2024
packages/@o3r/extractors/src/core/comparator/metadata-comparison.helper.ts
Outdated
Show resolved
Hide resolved
pginoux-1A
reviewed
May 23, 2024
| * @param granularity 'major' or 'minor' | ||
| */ | ||
| export function getVersionRangeFromLatestVersion(version: string, granularity: 'major' | 'minor'): string { | ||
| const semver = coerce(version); |
There was a problem hiding this comment.
Might be a little permissive, but not sure it's a big deal here (ex: semver.valid(semver.coerce("42.6.7.9.3-alpha")); // '42.6.7')
pginoux-1A
reviewed
May 23, 2024
...@o3r/localization/builders/metadata-check/helpers/localization-metadata-comparison.helper.ts
Outdated
Show resolved
Hide resolved
cpourcel
force-pushed
the
feature/metadata-migration-check
branch
from
105a0e8
to
a8852c0
Compare
packages/@o3r/extractors/src/core/comparator/metadata-comparator.interface.ts
Outdated
Show resolved
Hide resolved
packages/@o3r/extractors/src/core/comparator/metadata-comparison.helper.ts
Outdated
Show resolved
Hide resolved
packages/@o3r/extractors/src/core/comparator/metadata-comparison.helper.ts
Outdated
Show resolved
Hide resolved
packages/@o3r/extractors/src/core/comparator/metadata-comparison.helper.ts
Outdated
Show resolved
Hide resolved
packages/@o3r/extractors/src/core/comparator/metadata-comparison.helper.ts
Outdated
Show resolved
Hide resolved
packages/@o3r/extractors/src/core/comparator/metadata-comparison.helper.ts
Outdated
Show resolved
Hide resolved
cpourcel
force-pushed
the
feature/metadata-migration-check
branch
from
a8852c0
to
f234d6b
Compare
packages/@o3r/extractors/src/core/comparator/metadata-files.helper.ts
Outdated
Show resolved
Hide resolved
matthieu-crouzet
force-pushed
the
feature/metadata-migration-check
branch
2 times, most recently
from
d97f03e
to
67b9de7
Compare
vscaiceanu-1a
previously approved these changes
Jul 4, 2024
packages/@o3r/extractors/src/core/comparator/metadata-files.helper.ts
Outdated
Show resolved
Hide resolved
matthieu-crouzet
force-pushed
the
feature/metadata-migration-check
branch
6 times, most recently
from
7b11207
to
d1d1e8c
Compare
kpanot
reviewed
Jul 9, 2024
packages/@o3r/components/builders/metadata-check/helpers/config-metadata-comparison.helper.ts
Outdated
Show resolved
Hide resolved
packages/@o3r/components/builders/metadata-check/helpers/config-metadata-comparison.helper.ts
Show resolved
Hide resolved
| */ | ||
| const getLocalizationArray = (content: LocalizationMetadata) => content; | ||
|
|
||
| const getLocalizationName = (localization: JSONLocalization) => localization.key; |
There was a problem hiding this comment.
It's a be strange to have comment only on 1 const (even if they are not required because not exported)
packages/@o3r/extractors/src/core/comparator/metadata-files.helper.ts
Outdated
Show resolved
Hide resolved
...@o3r/extractors/src/core/comparator/package-managers-extractors/npm-file-extractor.helper.ts
Show resolved
Hide resolved
fpaul-1A
force-pushed
the
feature/metadata-migration-check
branch
3 times, most recently
from
c59ffc4
to
0d64b94
Compare
fpaul-1A
force-pushed
the
feature/metadata-migration-check
branch
2 times, most recently
from
0622812
to
2fbb35d
Compare
kpanot
reviewed
Jul 17, 2024
packages/@o3r/components/builders/metadata-check/helpers/config-metadata-comparison.helper.ts
Outdated
Show resolved
Hide resolved
| : [config] | ||
| ); | ||
|
|
||
| const getConfigurationPropertyName = (config: ComponentConfigOutput) => `${config.library}#${config.name}` + (config.properties.length ? ` ${config.properties[0].name}` : ''); |
There was a problem hiding this comment.
Missing comment (not needed but provided in getConfigurationArray)
...3r/extractors/src/core/comparator/package-managers-extractors/yarn2-file-extractor.helper.ts
Show resolved
Hide resolved
fpaul-1A
force-pushed
the
feature/metadata-migration-check
branch
2 times, most recently
from
9a7031b
to
4bc830d
Compare
mrednic-1A
reviewed
Jul 18, 2024
| * @returns {import('ts-jest/dist/types').JestConfigWithTsJest} | ||
| */ | ||
| module.exports.getJestProjectConfig = (rootDir, isAngularSetup) => { | ||
| module.exports.getJestProjectConfig = (rootDir, isAngularSetup, options) => { |
There was a problem hiding this comment.
nitpicking:
isAngularSetup could be included in options
maybe rootdir too .
fpaul-1A
approved these changes
Jul 18, 2024
mrednic-1A
approved these changes
Jul 19, 2024
This was referenced Jul 19, 2024
fpaul-1A
force-pushed
the
feature/metadata-migration-check
branch
from
4bc830d
to
255b3bf
Compare
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.
Proposed change
Add 3 new builders in @o3r/component, @o3r/styling and @o3r/localization, check-config-migration-metadata, check-localization-styling -metadata and check-localization-migration-metadata
The goal of these builders is to retrieve a previous version of the app (provided as an option or computed from the folder of migration metadata) and compare the current metadata with the previous one. It will raise some errors if breaking changes are detected and they are not allowed, or if these changes are not documented in the provided migration metadata.
Related issues