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 parser-js in most places #264

Merged

Conversation

magicmatatjahu
Copy link
Member

@magicmatatjahu magicmatatjahu commented Mar 17, 2021

Description

if someone think that the PR is too big, I will try to divide the PR into two smaller ones.

Changes proposed in this pull request:

  • Switch to parser-js in Info, Servers (also in single Server), Channels (also in single Channel), Messages (also in single Message), Schemas components,
  • left single Schema component, because it is the most complicated component in package and it should be rewritten with Render missed parts in react-component that are showed in HTML template shape-up-process#87 task - also by this I left some code related to "beautify" our schema - if Schema component will be rewritten, we will remove about leftover 10-15% of code.
  • I wanna treat PR as chore, because I wanna merge to unify-component branch :)

Related issue(s)
Part of asyncapi/shape-up-process#85

@magicmatatjahu magicmatatjahu added enhancement New feature or request area/library Related to all activities around Library package labels Mar 17, 2021
@sonarqubecloud
Copy link

Kudos, SonarCloud 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
No Duplication information No Duplication information

Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

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

Great stuff, Maciej! 👏

@fmvilas
Copy link
Member

fmvilas commented Mar 17, 2021

BTW, I think the beautifying process should be removed entirely. Not on this PR but at some point during the cycle. As it is right now, there are a couple of things that are not needed or incorrect:

  • Resolving allOf: this is incorrect, we're treating allOf as merging objects and that's not the correct behavior.
  • Rendering Markdown can be done using a React Markdown component.

@magicmatatjahu
Copy link
Member Author

magicmatatjahu commented Mar 17, 2021

@fmvilas

BTW, I think the beautifying process should be removed entirely. Not on this PR but at some point during the cycle. As it is right now, there are a couple of things that are not needed or incorrect:

Resolving allOf: this is incorrect, we're treating allOf as merging objects and that's not the correct behavior.
Rendering Markdown can be done using a React Markdown component.

I mentioned about removing this process in sentence 😄 :

...also by this I left some code related to "beautify" our schema - if Schema component will be rewritten, we will remove about leftover 10-15% of code.

I used React Markdown component in previous job and it's awesome, but this is a nice-to-have feature in unification :) We can stay with markdown-it library used here https://github.com/asyncapi/asyncapi-react/blob/master/library/src/helpers/renderMarkdown.tsx

@derberg
Copy link
Member

derberg commented Mar 18, 2021

@magicmatatjahu I would just change commit scope to refactor: and :shipit:

@magicmatatjahu magicmatatjahu changed the title chore: switch to parser-js in most places refactor: switch to parser-js in most places Mar 18, 2021
@magicmatatjahu
Copy link
Member Author

@derberg Done :)

@magicmatatjahu magicmatatjahu merged commit afa84a8 into asyncapi:unify-component Mar 18, 2021
@magicmatatjahu magicmatatjahu deleted the move-to-parser-js branch March 18, 2021 11:32
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 0.21.0-next.1 🎉

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
Labels
area/library Related to all activities around Library package enhancement New feature or request released on @next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants