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.

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

fix!: handle edge case in toCamelCase FormatHelper #1792

Merged
merged 4 commits into from
Feb 14, 2024

Conversation

moritzkalwa
Copy link
Contributor

@moritzkalwa moritzkalwa commented Feb 12, 2024

Description

The toCamelCase helper produced wrong outputs in some edge cases, where there was a numeric followed by "_" and a lowercase letter. This PR aims to fix that.

Related Issue

Fixes #1503

Checklist

  • The code follows the project's coding standards and is properly linted (npm run lint).
  • Tests have been added or updated to cover the changes.
  • Documentation has been updated to reflect the changes.
  • All tests pass successfully locally.(npm run test).

Copy link

netlify bot commented Feb 12, 2024

Deploy Preview for modelina canceled.

Name Link
🔨 Latest commit 40ac4c7
🔍 Latest deploy log https://app.netlify.com/sites/modelina/deploys/65cc9b340b5f4200093a927d

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@moritzkalwa moritzkalwa changed the title Handle edge case in toCamelCase FormatHelper fix: Handle edge case in toCamelCase FormatHelper Feb 12, 2024
@moritzkalwa moritzkalwa changed the title fix: Handle edge case in toCamelCase FormatHelper fix: handle edge case in toCamelCase FormatHelper Feb 12, 2024
@jonaslagoni
Copy link
Sponsor Member

@moritzkalwa this is gonna be a breaking change, let me setup the next branch so you can target it instead of master ✌️

@moritzkalwa
Copy link
Contributor Author

@moritzkalwa this is gonna be a breaking change, let me setup the next branch so you can target it instead of master ✌️

I am targeting the next branch already though 🤔

@jonaslagoni jonaslagoni changed the title fix: handle edge case in toCamelCase FormatHelper fix!: handle edge case in toCamelCase FormatHelper Feb 13, 2024
@jonaslagoni
Copy link
Sponsor Member

It is currently in an old state, updating it with #1795 then we can merge yours ✌️

Copy link
Sponsor Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

Do you mind adding a v3-v4 migration document and adding the camel case changes that break existing models?

@moritzkalwa
Copy link
Contributor Author

Do you mind adding a v3-v4 migration document and adding the camel case changes that break existing models?

Nope, all for it! Gonna try to get that done today :)

@jonaslagoni
Copy link
Sponsor Member

next is ready, after the final changes we can merge ✌️

@moritzkalwa
Copy link
Contributor Author

@jonaslagoni I added a migration guide. Hope that does it, most users should not encounter any issues anyway since this should be fairly rare and if they do all they can do is check for properties that were renamed.

Copy link
Sponsor Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

Since we are changing how camelCase works for all it's a more generic migration guide, so did some small changes ✌️

Other then that it looks good 💪

docs/migrations/version-3-to-4.md Outdated Show resolved Hide resolved
docs/migrations/version-3-to-4.md Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Feb 14, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@jonaslagoni
Copy link
Sponsor Member

/rtm

@jonaslagoni jonaslagoni merged commit 26bb9ba into asyncapi:next Feb 14, 2024
16 checks passed
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 4.0.0-next.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@jonaslagoni
Copy link
Sponsor Member

@all-contributors please add @moritzkalwa for code, test, bug, docs

Copy link
Contributor

@jonaslagoni

I've put up a pull request to add @moritzkalwa! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants