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

feat: exclude underscore from TS key enum rendering #613

Merged
merged 5 commits into from
Feb 8, 2022

Conversation

Ishan-Saini
Copy link
Contributor

@Ishan-Saini Ishan-Saini commented Feb 3, 2022

Description

  • Added _ to the exclusion list inside TS key enum renderer.
  • Added tests for EnumRenderer.

Related issue(s)
Resolves #609

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.

@Ishan-Saini
Copy link
Contributor Author

Hey @jonaslagoni , I am not sure if this falls under the chore: specification.
Also, let me know if something needs to be changed.

@coveralls
Copy link

coveralls commented Feb 3, 2022

Pull Request Test Coverage Report for Build 1811933408

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.542%

Totals Coverage Status
Change from base Build 1811568139: 0.0%
Covered Lines: 2459
Relevant Lines: 2505

💛 - Coveralls

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.

Hey @jonaslagoni , I am not sure if this falls under the chore: specification.
Also, let me know if something needs to be changed.

You can use feat here 🙂

I can see we did not create a test for the EnumRenderer... Do you mind creating a test file for it under test/generators/typescript/renderers/EnumRenderer.spec.ts? 🙏

For test cases, all you need to test is normalizeKey:

  • should correctly convert underscore character.
    • Given something_something you expect SOMETHING_SOMETHING to be returned.
  • should correctly convert space character.
    • Given something something you expect SOMETHING_SOMETHING to be returned.

Feel free to add more cases if you feel like it 🙂

src/generators/typescript/renderers/EnumRenderer.ts Outdated Show resolved Hide resolved
@Ishan-Saini Ishan-Saini changed the title chore: exclude underscore from TS key enum rendering feat: exclude underscore from TS key enum rendering Feb 4, 2022
@Ishan-Saini
Copy link
Contributor Author

Ishan-Saini commented Feb 4, 2022

I can see we did not create a test for the EnumRenderer... Do you mind creating a test file for it under test/generators/typescript/renderers/EnumRenderer.spec.ts? pray

I am new to testing but I'll still try to get it done asap. 😅

@jonaslagoni
Copy link
Member

jonaslagoni commented Feb 4, 2022

I am new to testing but I'd still try to get it done asap.

No worries!

I can give you the core testing setup if you want that, that way you only need to add the tests for the function, let me know 🙂

Otherwise ping me if you get stuck 👍

@Ishan-Saini
Copy link
Contributor Author

I can give you the core testing setup if you want that, that way you only need to add the tests for the function, let me know slightly_smiling_face

I'll try to get it done myself first.
Will let you know if I get stuck :)

@Ishan-Saini
Copy link
Contributor Author

Ishan-Saini commented Feb 6, 2022

I can give you the core testing setup if you want that, that way you only need to add the tests for the function, let me know slightly_smiling_face

Hey @jonaslagoni , Could you please provide the core testing setup 😬?
A bit stuck, I think the setup would help me get started.

@jonaslagoni
Copy link
Member

A bit stuck, I think the setup would help me get started.

For sure!

Here you go @Ishan-Saini (test/generators/typescript/renderers/EnumRenderer.spec.ts):

import { TypeScriptGenerator } from '../../../../src/generators';
import { EnumRenderer } from '../../../../src/generators/typescript/renderers/EnumRenderer';
import { CommonInputModel, CommonModel } from '../../../../src/models';

describe('TypeScriptRenderer', () => {
  let renderer: EnumRenderer;
  beforeEach(() => {
    renderer = new EnumRenderer(TypeScriptGenerator.defaultOptions, new TypeScriptGenerator(), [], new CommonModel(), new CommonInputModel());
  });

  describe('normalizeKey()', () => {
    test('should correctly format " " to correct key', () => {
      
    });
    test('should correctly format "_" to correct key', () => {
      
    });
  });
});

@Ishan-Saini
Copy link
Contributor Author

@jonaslagoni
Thank you for providing the setup, it really cleared a lot of things for me. I have pushed the changes, let me know if something is wrong.

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.

Small change.

Also, I see you updated package-lock.json, do you mind reverting it to the current one? Just copy-paste it from masterand commit it 🙂

It is a known "problem" with never versions of node, but our CI uses older versions, so we need to keep it to their liking (for now).

test/generators/typescript/renderers/EnumRenderer.spec.ts Outdated Show resolved Hide resolved
@Ishan-Saini
Copy link
Contributor Author

Also, I see you updated package-lock.json, do you mind reverting it to the current one? Just copy-paste it from masterand commit it slightly_smiling_face

It is a known "problem" with never versions of node, but our CI uses older versions, so we need to keep it to their liking (for now).

Oh okay, I will keep that in mind :)

@sonarcloud
Copy link

sonarcloud bot commented Feb 8, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

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.

LGTM 🎉

@jonaslagoni
Copy link
Member

/rtm

@asyncapi-bot asyncapi-bot merged commit 388495e into asyncapi:master Feb 8, 2022
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 0.47.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 1.0.0-next.23 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

Exclude _ from TypeScript key enum rendering
4 participants