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

Go package #118

Closed
smoya opened this issue Nov 1, 2021 · 9 comments · Fixed by #124
Closed

Go package #118

smoya opened this issue Nov 1, 2021 · 9 comments · Fixed by #124
Labels
enhancement New feature or request

Comments

@smoya
Copy link
Member

smoya commented Nov 1, 2021

Reason/Context

I’m trying to give some 💛 to https://github.com/asyncapi/parser-go. I just wanted to add support for the latest spec versions (it only supports 2.0.0).
In order to get the JSON Schema files for each spec, so it supports all of them, we would need to follow a similar approach @asyncapi/specs` package does (built from this repository).

Description

I suggest we just add a .go file into this repository similar to index.js.
I have a POC here https://github.com/smoya/spec-json-schemas/blob/feat/go/spec-json-schemas.go

@smoya smoya added the enhancement New feature or request label Nov 1, 2021
@github-actions
Copy link

github-actions bot commented Nov 1, 2021

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup 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.

@fmvilas
Copy link
Member

fmvilas commented Nov 1, 2021

Big yes from me 🚀 This repo is not meant to be Node.js only. Actually, we changed its name from asyncapi-node to spec-json-schemas for this reason.

@derberg
Copy link
Member

derberg commented Nov 2, 2021

Great idea. I'm only afraid of putting it next to index.js. Won't it cause a mess? Shouldn't we just start creating subfolders, node, go, and then next ones?

@smoya
Copy link
Member Author

smoya commented Nov 2, 2021

Great idea. I'm only afraid of putting it next to index.js. Won't it cause a mess? Shouldn't we just start creating subfolders, node, go, and then next ones?

I thought the same. Then I realized only one .go file is needed (along with one _test.go), also that most of CI tooling is configured for detecting package.lock and go.sum or similar on the root.
Perhaps we could consider moving to subfolders if we ever give support to more parsers written in different languages?

@derberg
Copy link
Member

derberg commented Nov 3, 2021

oh, you are in a rush here 😅

  • actually, you have 4 not 2 new files now on the root
  • monorepo is pretty known these days and most of CI tools are configurable enough to point to files located in different directories
  • readme was not updated
  • I did not notice any new config in package.json so I'm pretty sure all these not node related files will be from now on included in the npm package 😄
  • and last but not least, why did we release a new npm package version anyway?

@smoya
Copy link
Member Author

smoya commented Nov 3, 2021

I agree I rushed a bit on this issue. I didn't consider it to be that harmful as you probably did. Otherwise, I would have waited for more feedback obviously!

I did not notice any new config in package.json so I'm pretty sure all these not node related files will be from now on included in the npm package 😄

I don't know why I assumed they won't be included. My fault.

and last but not least, why did we release a new npm package version anyway?
Considering the nature of both packages, including the fact they should not change a lot, I think moving into a full monorepo strategy, with CI/CD can add too much complexity.
You say most CI tools are configurable somehow. Yes, I agree, for example, GH workflows can be configured for being triggered only when some paths are affected. However, the rest of the release flow will be on us, unless we introduce big tools such as Bazel, etc. (if you know any tool that is simple to use, just drop a comment).

For that reason, I decided to go with the most simple way of handling this: Symmetric releases for all packages.
I agree releasing a new package without introducing changes is not ideal. But I don't expect that to happen that much.

What's your suggestion in terms of releasing strategy for this monorepo? At a glance, I see the following concerns:

  • A change in schemas should always release new packages for both Nodejs and Go.
  • A change in Nodejs package should release a new Nodejs package.
  • A change in Go package should release a new Go package.
  • GH release tags should include a prefix with the language name: nodejs-v2.20.1, go-v1.0.1
  • Package versions (npm, go module) should remove the language prefix from the released tag. From examples above, npm:v2.20.1, go: v1.0.1.

@derberg
Copy link
Member

derberg commented Nov 4, 2021

For that reason, I decided to go with the most simple way of handling this: Symmetric releases for all packages.

this must be clear when we do changes, when we discuss issues, when we open PRs. I'm sure that Fran approved without knowing this as it was not mentioned.

@asyncapi/spec is downloaded already over 1M times and we should always discuss in group so important things like changes in the release.

This package is used by the parser, and therefore by every single tool we have in JS. So basically an empty release of this package will cause an empty release in the entire AsyncAPI ecosystem. I have to say I definitely do not like it 😄 even if this is just from time to time. Empty releases are evil IMHO. Symetric release is cool in react component where we release different bundles and versions of it, but here, not really.

so yeah, this is what I meant by rushing.

What's your suggestion in terms of releasing strategy for this monorepo?

maybe in the end it was not the best idea to do a monorepo for this project 🤷🏼 intentions made sense but from release perspective it sounds unreasonable.

@smoya
Copy link
Member Author

smoya commented Nov 4, 2021

I love your explanation with so much detail about what you were concern of. Really it means a lot 💯
I see the point and can understand your pov.

For that reason, I decided to go with the most simple way of handling this: Symmetric releases for all packages.

this must be clear when we do changes, when we discuss issues, when we open PRs. I'm sure that Fran approved without knowing this as it was not mentioned.

@asyncapi/spec is downloaded already over 1M times and we should always discuss in group so important things like changes in the release.

This package is used by the parser, and therefore by every single tool we have in JS. So basically an empty release of this package will cause an empty release in the entire AsyncAPI ecosystem. I have to say I definitely do not like it 😄 even if this is just from time to time. Empty releases are evil IMHO. Symetric release is cool in react component where we release different bundles and versions of it, but here, not really.

so yeah, this is what I meant by rushing.

What's your suggestion in terms of releasing strategy for this monorepo?

maybe in the end it was not the best idea to do a monorepo for this project 🤷🏼 intentions made sense but from release perspective it sounds unreasonable.

Maybe the release effort is too much so indeed it is not worth to maintain a monorepo. If we don't do monorepo, we will either need to maintain a mirror copy of the schemas on each repository, or use git submodules.

@derberg
Copy link
Member

derberg commented Nov 10, 2021

Really it means a lot 💯

Sounds strange but happy to help 😆

Maybe the release effort is too much so indeed it is not worth to maintain a monorepo. If we don't do monorepo, we will either need to maintain a mirror copy of the schemas on each repository, or use git submodules.

This is basically the whole point I have. There is no one best solution and we need to remember to discuss first before we make changes that affect stuff like release process.

Like for example now it all looks to me like basically a move of schemas to spec-json-schemas was not the best, that we should probably move them back again to spec repo, and have repo per language where we have packages with mirror copies of schemas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants