Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(util): add normalizeIdentifier function #606

Merged
merged 6 commits into from
Feb 21, 2023
Merged

feat(util): add normalizeIdentifier function #606

merged 6 commits into from
Feb 21, 2023

Conversation

mttrbrts
Copy link
Member

@mttrbrts mttrbrts commented Feb 15, 2023 •
edited
Loading

Adds a new function called normalizeIdentifier to the @accordproject/concerto-util package. This function allows arbitrary values to be munged into valid Concerto identifiers.

This is useful when accepting user-provided values while dynamically building models.

Changes

  • Adds new module and function normalizeIdentifier and exhaustive tests

Author Checklist

  • Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
  • Vital features and changes captured in unit and/or integration tests
  • Commits messages follow AP format
  • Extend the documentation, if necessary
  • Merging to main from fork:branchname

@mttrbrts
feat(util): add normalizeIdentifiers function
22c8298 
Signed-off-by: Matt Roberts <[email protected]>

Update packages/concerto-util/lib/identifiers.js

wip

Signed-off-by: Matt Roberts <[email protected]>

add docs + more tests

Signed-off-by: Matt Roberts <[email protected]>

add truncation tests

Signed-off-by: Matt Roberts <[email protected]>

upgrade eslint

Signed-off-by: Matt Roberts <[email protected]>

generate typescript types

Signed-off-by: Matt Roberts <[email protected]>

upgrade typescript add @types/node

Signed-off-by: Matt Roberts <[email protected]>
@mttrbrts mttrbrts force-pushed the mr-normalize-identifiers branch from e38a9b1 to 22c8298 Compare February 18, 2023 16:14
@mttrbrts mttrbrts marked this pull request as ready for review February 18, 2023 17:45
@mttrbrts mttrbrts requested a review from a team February 18, 2023 17:46
@mttrbrts mttrbrts enabled auto-merge (squash) February 18, 2023 17:47
dselman
dselman reviewed Feb 21, 2023
View reviewed changes
Copy link
Contributor

@dselman dselman left a comment

Choose a reason for hiding this comment

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

Couple of minor comments.

packages/concerto-util/lib/identifiers.js Outdated Show resolved Hide resolved
packages/concerto-util/lib/identifiers.js Show resolved Hide resolved
packages/concerto-util/lib/identifiers.js Show resolved Hide resolved
@mttrbrts mttrbrts disabled auto-merge February 21, 2023 08:52
@mttrbrts mttrbrts enabled auto-merge (squash) February 21, 2023 09:08
@mttrbrts mttrbrts requested a review from dselman February 21, 2023 09:12
@mttrbrts
chore: code comment
0d1913f 
Signed-off-by: Matt Roberts <[email protected]>
dselman
dselman approved these changes Feb 21, 2023
View reviewed changes
Copy link
Contributor

@dselman dselman left a comment

Choose a reason for hiding this comment

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

Great. Love the tests!

@mttrbrts mttrbrts merged commit 690ce26 into accordproject:main Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dselman dselman dselman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mttrbrts @dselman