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

ci: publish playground from next branch #352

Merged

Conversation

magicmatatjahu
Copy link
Member

@magicmatatjahu magicmatatjahu commented May 25, 2021

Description

Changes proposed in this pull request:

  • publish playground from next branch - update appropriate workflow for that
  • improve showing errors from parser: break the words

Errors:
image

Related issue(s)
Part of #300

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.

correct me if I'm wrong, but this PR introduces features and fixes for 3 totally unrelated topics, right?

  • update of openapi-sampler should be a separate PR to master and merged as feat: with a proper description of what else is added with their first major version of openapi-sampler, as it is purely not only because of time format
  • fix of errors should also go to master

above can be then pulled into next

and last but not least is a PR to next with circular refs support for items as this is a part of the release candidate for major version.

@magicmatatjahu
Copy link
Member Author

magicmatatjahu commented May 26, 2021

@derberg Thanks for the review!

update of openapi-sampler should be a separate PR to master and merged as feat: with a proper description of what else is added with their first major version of openapi-sampler, as it is purely not only because of time format

Ok, I will create a PR for that. There is also supporting for draft-07 of JSON Schema and supports fo other formats than time.

fix of errors should also go to master
above can be then pulled into next

It cannot be done, and then pulled, because code for displaying errors is different in next and master branches, it's not only related code itself, but also for different styling fiori and tailwind. We cannot reconcile it. Also easy bumping the openapi-sampler package - we will have a lot of changes in package-lock between branches.

TBH I don't think so that we should split those changes to 3 separates PRs, then it's is an art for art, but ok, I will create it.

@magicmatatjahu
Copy link
Member Author

In addition, errors from parser has been displaying on the master for a long time, I just want to close one issue, because I fixed some styles displaying errors -> little change https://github.com/asyncapi/asyncapi-react/pull/352/files#diff-18b2a4882a0019cfc814b13ccea23aeaf24e1835040ea9912df58f980043aa0cR19 so I don't think so that we should split this change to separate PR.

@magicmatatjahu magicmatatjahu changed the title fix: support circular refs for items ci: publish playground from next branch May 26, 2021
@derberg
Copy link
Member

derberg commented May 26, 2021

@magicmatatjahu not really art for art, it is just cleaner and by pushing openapi-sampler to master you make the feature available to others long weeks before major version of the component is available, it is worth it. As for the errors, I agree that if it is going to be to complex later to pull it into next, then let us not put it into master

@magicmatatjahu
Copy link
Member Author

not really art for art, it is just cleaner and by pushing openapi-sampler to master you make the feature available to others long weeks before major version of the component is available, it is worth i

About art for art was related to splitting PRs to 3 separate for next :P I agree with bumping package also in master.

@derberg
Copy link
Member

derberg commented May 26, 2021

About art for art was related to splitting PRs to 3 separate

if you want to have a quick review, then it is not art 🤷🏼
just look at what you've done here -> #356. This is just beautiful dude, one file changed, PR focused on one thing, I do not have to spend too much time on figuring out what changes is related to what 1 of 3 issues 🤷🏼 it is art for me, but a positive art, I hope you see it 😄

@sonarcloud
Copy link

sonarcloud bot commented May 26, 2021

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

@magicmatatjahu
Copy link
Member Author

@derberg Thanks a lot!

@magicmatatjahu magicmatatjahu merged commit ed998f9 into asyncapi:next May 26, 2021
@magicmatatjahu magicmatatjahu deleted the next-branches/update-deps branch May 26, 2021 12:00
@asyncapi-bot
Copy link
Contributor

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

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.

3 participants