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.

Sign up for GitHub

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

Jump to bottom

refactor: update Readme.md v1 #601

Merged
merged 6 commits into from
Sep 12, 2022
Merged

refactor: update Readme.md v1 #601

merged 6 commits into from
Sep 12, 2022

Conversation

magicmatatjahu
Copy link
Member

Description

Update Readme.md with informations:

  • add installation guide
  • add examples
  • update custom parsers section
  • update stringify section
  • update develop section

Other section will be updated in next PRs.

cc @smoya

Related issue(s)
Part of #481
Part of #482

@magicmatatjahu magicmatatjahu added enhancement New feature or request 📑 docs labels Sep 8, 2022
@magicmatatjahu
Copy link
Member Author

markdown link workflow throws error:

FILE: README.md
[✖] /docs/api.md → Status: 400 [Error: ENOENT: no such file or directory, access '/docs/api.md'] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'access',
  path: '/docs/api.md'
}

But we don't have such a link in this file, lol 😆

smoya
smoya reviewed Sep 9, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
smoya
smoya reviewed Sep 9, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
smoya
smoya reviewed Sep 9, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
smoya
smoya reviewed Sep 9, 2022
View reviewed changes
README.md Show resolved Hide resolved
smoya
smoya requested changes Sep 9, 2022
View reviewed changes
Copy link
Member

@smoya smoya left a comment

Choose a reason for hiding this comment

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

Looks pretty good! Just some minor changes.

smoya
smoya reviewed Sep 9, 2022
View reviewed changes
README.md
Comment on lines +129 to +147
asyncapi: '2.4.0'
info:
title: Example AsyncAPI specification
version: '0.1.0'
channels:
example-channel:
subscribe:
message:
payload:
type: object
properties:
exampleField:
type: string
exampleNumber:
type: number
exampleDate:
type: string
format: date-time
`);
Copy link
Member

Choose a reason for hiding this comment

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

This example is not valid to Spectral because of this rule asyncapi-operation-operationId: https://github.com/stoplightio/spectral/blob/297531bbc6f26393d325a101b59f9e8b1f57e21e/packages/rulesets/src/asyncapi/index.ts#L230-L240

Not sure if we really want to keep that rule there. The operationId is not mandatory by the spec.

Copy link
Member Author

Choose a reason for hiding this comment

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

I know about some "problems" with Spectral. Now I would leave that spec, because at the end operationId won't be mandatory on Spectral side, don't worry.

Copy link
Member

Choose a reason for hiding this comment

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

Do you mean that particular spectral rule will be gone? Could you bring more details, please?

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed will be severity from error to warning - then there will be information that it is a good pattern design. Most of the rules from Spectral were made 2 years ago by the Stoplight team itself and a lot of things are not up to date. This week I have a plan to go through all the rules and improve them.

Copy link
Member

Choose a reason for hiding this comment

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

Changed will be severity from error to warning

That makes totally sense to me. I have few concerns regarding this but I'm not gonna add anything extra into this issue, but rather please ping me on the work you plan to do 🙏

Copy link
Member Author

Choose a reason for hiding this comment

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

I will create PR with that in Spectral side, so maybe better will be if you will list everything bugs that you see, hmm?

@smoya smoya mentioned this pull request Sep 9, 2022
Merged
magicmatatjahu and others added 2 commits September 12, 2022 11:03
@magicmatatjahu @smoya
Apply suggestions from code review
472376a 
Co-authored-by: Sergio Moya <1083296+smoya@users.noreply.github.com>
@magicmatatjahu
Merge branch 'next-major' into next/update-readme-v1
80e2b54
@sonarcloud
Copy link

sonarcloud bot commented Sep 12, 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
No Duplication information No Duplication information

smoya
smoya approved these changes Sep 12, 2022
View reviewed changes
Copy link
Member

@smoya smoya left a comment

Choose a reason for hiding this comment

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

LGTM!

@magicmatatjahu
Copy link
Member Author

/rtm

@asyncapi-bot asyncapi-bot merged commit 71e8672 into asyncapi:next-major Sep 12, 2022
@magicmatatjahu magicmatatjahu deleted the next/update-readme-v1 branch September 12, 2022 12:26
@magicmatatjahu magicmatatjahu mentioned this pull request Sep 14, 2022
Closed
20 tasks
magicmatatjahu added a commit to magicmatatjahu/parser-js that referenced this pull request Oct 3, 2022
@magicmatatjahu
refactor: update Readme.md v1 (asyncapi#601)
43bf0df
derberg pushed a commit that referenced this pull request Oct 4, 2022
@magicmatatjahu @derberg
refactor: update Readme.md v1 (#601)
c36194c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smoya smoya smoya approved these changes

@fmvilas fmvilas Awaiting requested review from fmvilas fmvilas is a code owner

@jonaslagoni jonaslagoni Awaiting requested review from jonaslagoni jonaslagoni is a code owner

@derberg derberg Awaiting requested review from derberg derberg is a code owner

@asyncapi-bot-eve asyncapi-bot-eve Awaiting requested review from asyncapi-bot-eve asyncapi-bot-eve is a code owner

Assignees
No one assigned
Labels
📑 docs enhancement New feature or request ready-to-merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@magicmatatjahu @smoya @asyncapi-bot