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

refactor: switch to optional chaining in custom filters and update eslint config #1274

Merged
merged 4 commits into from
Oct 8, 2024

Conversation

rishabhr4
Copy link
Contributor

Description
This pull request addresses the issue of unnecessary escape characters in the code.

Changes Made
Removed unnecessary escape characters ($) from the fileName.replace statement:

Removed Unnecessary character escapes
Copy link

changeset-bot bot commented Oct 3, 2024

⚠️ No Changeset found

Latest commit: 44ce244

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

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.

@Gmin2
Copy link
Collaborator

Gmin2 commented Oct 3, 2024

please also include the sonarcloud error that this PR target to solve, thanks cc @rishabhr4

@rishabhr4
Copy link
Contributor Author

rishabhr4 commented Oct 3, 2024

Description This pull request addresses the issue of unnecessary escape characters in the code.

Changes Made Removed unnecessary escape characters ($) from the fileName.replace statement:

This PR resolves this issue.

used optional chain expression
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

please remove changes from apps/generator/lib/generator.js as I don't think we have that covered with tests, and some more work will be needed - so yeah, let us not mix the scope, please add separate PR for that

@derberg
Copy link
Member

derberg commented Oct 7, 2024

@rishabhr4 please react to failing test run

updated ecam version so that the failing linting test runs
Copy link

sonarcloud bot commented Oct 8, 2024

@rishabhr4
Copy link
Contributor Author

@rishabhr4 please react to failing test run

Done cc @derberg

@derberg
Copy link
Member

derberg commented Oct 8, 2024

what was the reason for changing eslint config?

@derberg derberg changed the title chore: remove unnecessary escape characters in fileName.replace statement refactor: switch to optional chaining in custom filters and update eslint config Oct 8, 2024
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

thanks a lot

@derberg
Copy link
Member

derberg commented Oct 8, 2024

ok, nevermind I figured it needs an update as old linter did not support optional chaining

@derberg
Copy link
Member

derberg commented Oct 8, 2024

/rtm

@asyncapi-bot asyncapi-bot merged commit 5c28787 into asyncapi:master Oct 8, 2024
15 checks passed
@rishabhr4
Copy link
Contributor Author

ok, nevermind I figured it needs an update as old linter did not support optional chaining

yeah that's why linter tests were failing

@rishabhr4 rishabhr4 deleted the rishabhdev branch October 8, 2024 16:29
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.

4 participants