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: make parameter type of ToEnum extension methods dynamic #2064

Merged

Conversation

jespitae
Copy link

@jespitae jespitae commented Jun 26, 2024

Description

Changed parameter type of ToEnum extension methods to dynamic.

Related Issue

fixes #2063

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).

Additional Notes

Copy link

netlify bot commented Jun 26, 2024

Deploy Preview for modelina canceled.

Name Link
🔨 Latest commit cc030a3
🔍 Latest deploy log https://app.netlify.com/sites/modelina/deploys/669513ed4008ff0008490fa3

@jonaslagoni jonaslagoni changed the title Make parameter type of ToEnum extension methods dynamic. fix: make parameter type of ToEnum extension methods dynamic Jun 26, 2024
Copy link
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.

Make sure to update test npm run test:update 🙂

@jespitae
Copy link
Author

Make sure to update test npm run test:update 🙂

Hi @jonaslagoni,

Running npm run test:update results in all the snapshots being updated, which seems incorrect:
image
For example:
image
I guess there is something wrong with my development setup?

@jonaslagoni
Copy link
Member

Make sure to update test npm run test:update 🙂

Hi @jonaslagoni,

Running npm run test:update results in all the snapshots being updated, which seems incorrect: image For example: image I guess there is something wrong with my development setup?

I have seen this before, but I cant remember what did it. Think I cleared cache and installed dependencies again 🤔

@jespitae
Copy link
Author

jespitae commented Jul 1, 2024

Make sure to update test npm run test:update 🙂

Hi @jonaslagoni,
Running npm run test:update results in all the snapshots being updated, which seems incorrect: image For example: image I guess there is something wrong with my development setup?

I have seen this before, but I cant remember what did it. Think I cleared cache and installed dependencies again 🤔

I cleaned my node_modules folder, cleared the cache and ran an npm install, but I still get the same result. Anything else I could try?

@jonaslagoni
Copy link
Member

What node version are you using? 🤔

@jespitae
Copy link
Author

jespitae commented Jul 3, 2024

What node version are you using? 🤔

image

@jonaslagoni
Copy link
Member

@jespitae try downgrade to v18 and clear cache, that is what I am currently using.

@jespitae
Copy link
Author

@jespitae try downgrade to v18 and clear cache, that is what I am currently using.

Hey,

I tried downgrading to v18. Same result still.
image

@jonaslagoni
Copy link
Member

@jespitae i am gonna try pull your changes and see if i get the same problem. Will do it later tonight ✌️

@jonaslagoni
Copy link
Member

jonaslagoni commented Jul 24, 2024

Finally had some time to sit down and try and reproduce it.

What system are you using @jespitae? Windows? That is where I can reproduce it.

@jonaslagoni
Copy link
Member

Looks like I have found a "bug" in the core of the Modelina models... How the hell that bug has not explicitly shown up before puzzles me... It looks like a small discrepancy between windows and linux/mac node/typescript/npm something that makes it show up there 🤨

@jespitae
Copy link
Author

Finally had some time to sit down and try and reproduce it.

What system are you using @jespitae? Windows? That is where I can reproduce it.

Indeed it's on windows.

@jonaslagoni
Copy link
Member

@jespitae can you try update your branch with the latest next branch?

@jonaslagoni jonaslagoni changed the base branch from next to dynamic-enum-csharp August 13, 2024 09:08
@jonaslagoni jonaslagoni merged commit b188a3a into asyncapi:dynamic-enum-csharp Aug 13, 2024
12 of 17 checks passed
@jonaslagoni
Copy link
Member

@jespitae I am gonna quickly fix it and merge it in another branch. Thanks for taking the time and sorry for the long time it took.

@jonaslagoni
Copy link
Member

@all-contributors please add @jespitae for code, bug

Copy link
Contributor

@jonaslagoni

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

jonaslagoni added a commit that referenced this pull request Aug 13, 2024
…2089)

* fix: make parameter type of ToEnum extension methods dynamic (#2064)

Co-authored-by: jespitae <[email protected]>
Co-authored-by: Jens Spitaels <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants