Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

fix: work correctly on pre-parsed AsyncAPI docs when given as Input #103

Merged

Conversation

arjungarg07
Copy link
Collaborator

Description

  • Earlier, we were not able to provide pre-parsed AsyncAPI documents as input. The bug has been fixed.

Related issue(s)
#74

@coveralls
Copy link

coveralls commented Feb 10, 2022

Pull Request Test Coverage Report for Build 2315283440

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+1.4%) to 93.333%

Totals Coverage Status
Change from base Build 2315059073: 1.4%
Covered Lines: 111
Relevant Lines: 114

💛 - Coveralls

Copy link

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

Looks good to me 👍 Want to add tests to ensure it always works?

Copy link
Collaborator

@magicmatatjahu magicmatatjahu left a comment

Choose a reason for hiding this comment

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

Yeah, we should add at least two simple tests, one with parsed AsyncAPI and another one with document saved string/JSON object.

@arjungarg07
Copy link
Collaborator Author

shall I replace the current tests with pre-parsed AsyncAPI docs or add tests for pre-parsed AsyncAPI docs separately.

@jonaslagoni
Copy link

shall I replace the current tests with pre-parsed AsyncAPI docs or add tests for pre-parsed AsyncAPI docs separately.

Depends on what you want to support, if it is both inputs, you would need two tests 🙂

lib/utils.js Outdated Show resolved Hide resolved
test/testsUtil.js Outdated Show resolved Hide resolved
arjungarg07 and others added 2 commits March 7, 2022 23:38
Co-authored-by: Maciej Urbańczyk <[email protected]>
Co-authored-by: Maciej Urbańczyk <[email protected]>
magicmatatjahu
magicmatatjahu previously approved these changes Mar 15, 2022
Copy link
Collaborator

@magicmatatjahu magicmatatjahu left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@arjungarg07
Copy link
Collaborator Author

@jonaslagoni @derberg shall we merge this one?

@magicmatatjahu
Copy link
Collaborator

@arjungarg07 I didn't notice that. You should fix the errors from sonarcloud.

@arjungarg07
Copy link
Collaborator Author

arjungarg07 commented Mar 24, 2022

@magicmatatjahu sorry, but I can't see any sonarcloud errors. Can you please point them out.

@jonaslagoni
Copy link

@arjungarg07 as code owner, remember you can always merge it your self ones all required checks passes 🙂 Use /rtm or /help for more information.

@arjungarg07
Copy link
Collaborator Author

@jonaslagoni since the issue was created by you, I was waiting for a review from your side as well😅

@jonaslagoni
Copy link

@jonaslagoni since the issue was created by you, I was waiting for a review from your side as well😅

Ahh okay, yea, keep pinging me then 😆 Cause once I see @magicmatatjahu reviewed it I did not expect I needed to spend time on it as well 🙂

jonaslagoni
jonaslagoni previously approved these changes Mar 24, 2022
Copy link

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

👍

@derberg
Copy link
Collaborator

derberg commented Apr 5, 2022

There are still sonarcloud issues

Screenshot 2022-04-05 at 10 26 57

@arjungarg07 arjungarg07 dismissed stale reviews from jonaslagoni and magicmatatjahu via b0c634b April 8, 2022 03:11
@arjungarg07
Copy link
Collaborator Author

@derberg one approving review required before merging.

@sonarqubecloud
Copy link

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

@arjungarg07 arjungarg07 merged commit 133921a into asyncapi-archived-repos:master May 12, 2022
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 0.6.16 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

6 participants