asyncapi / spec-json-schemas

AsyncAPI schema versions
Apache License 2.0
54 stars 54 forks source link

Acceptance criteria for new maintainers #513

Open smoya opened 6 months ago

smoya commented 6 months ago

There is no rule written in stone, but as far as I have seen, we always advocated for requiring new maintainers of this repo to also be maintainers of the Spec and ideally the Parser-JS (and Bindings?). Even though I believe it can make sense due to the strong dependency between those projects, I started recently questioning if we should keep doing this.

I found out several cases where changes (fixes, improvements...) are only required in this repo, not being accompanied by any change in the Spec or Parser. For example, changes in the bundler, fixes in the Schemas, etc. The last example, the one @Pakisan worked in https://github.com/asyncapi/spec-json-schemas/pull/495. In this case, some maintainers did the review (me included), however one of the deepest reviews it got was made by @jonaslagoni, who is not maintainer. This is not the first time it happens.

I wondered why @jonaslagoni is not maintainer of this repository, so I asked Jonas first. Its answer is pretty clear:

The main problem has always been I did not want to become codeowner of the spec as well.

Then I asked Jonas:

Would you step in if that constraint gets removed? (no need to be spec owner)

And the answer I got was a YES.

Call to action

I suggest we remove such a constraint (again I believe never written in stone) and accept new maintainers such as @jonaslagoni or @Pakisan.

@fmvilas @derberg @dalelane @char0n @GreenRover @mboss37 @GeraldLoeffler @rcoppen @SrfHead @lbroudoux @whitlockjc @damaru-inc @CameronRushton @VisualBean @dpwdec @iancooper @KhudaDad414 Please react to this with πŸ‘ if you agree, πŸ‘Ž if you disagree or πŸ‘€ if you don't want to vote but want to signal that you're aware and have read this.

Discussion is open, and happy to read your opinion on this πŸ™Œ

derberg commented 6 months ago

yup, agree.

actually I would suggest we rebuild codeowners file here entirely as we added also all binding owners here as well and as result, even if we invite @jonaslagoni or @Pakisan, then still bindings schemas owners will need to approve some PRs.

we should treat spec-json-schemas as a tool, and focus more to have JSON Schema experts here, rather then spec owners as anyway, whatever is in JSON Schema must be first added to the binding.

actually I would suggest we rebuild codeowners file here entirely

so give people like 2 weeks to react, and if no reaction, just refresh codeowners file. I can even take care for email contact asking people to look into this PR

dpwdec commented 6 months ago

Will current spec owners continue to manage their respective specs in the new codeowners? Or is the intention to remove the category entirely? Or just to introduce a β€œgeneral” maintainer category?

derberg commented 6 months ago

@smoya proposal is just to add @jonaslagoni and @Pakisan to the list of core/general maintainers (to change the rule that only https://github.com/asyncapi/spec maintainers can be maintainers here in this repo)

and I suggest that probably we should go back to what we had before, and have only https://github.com/asyncapi/spec-json-schemas/blob/master/CODEOWNERS#L9 without all additional binding-level codeowners. JSON Schema is basically a tool, so any improvements to schemas for bindings will require approval from all binding owners - which makes change process a hell. I think it is enough to have in contributing guide a rule that maintainer have to accept any PR that is making changes in schemas to reflect https://github.com/asyncapi/bindings - and then binding owner do not have to be actually code owner for JSON Schema. Not sure if that makes sense πŸ˜„

smoya commented 6 months ago

a rule that maintainer have to accept any PR that is making changes in schemas to reflect https://github.com/asyncapi/bindings

It makes sense to me.

WDYT @dpwdec? Just to clarify, in your particular case, you will still keep ownership of the SQS and SNS bindings in https://github.com/asyncapi/bindings.

smoya commented 6 months ago

@derberg Do you think it makes sense to create a new issue to let the rest of maintainers to vote about your suggestion and leave this one for yei or nei to @jonaslagoni and @Pakisan joining as maintainers? It already have some πŸ‘ .

derberg commented 6 months ago

@smoya yeah definitely, lets not keep them waiting

smoya commented 6 months ago

@jonaslagoni and @Pakisan, please feel free to open a PR adding yourself as maintainer to the CODEOWNERS file!

jonaslagoni commented 6 months ago

Under which existing sections (or new) are we adding ourselves? πŸ€”

smoya commented 6 months ago

Under which existing sections (or new) are we adding ourselves? πŸ€”

I believe in general. Line 9 https://github.com/asyncapi/spec-json-schemas/blob/master/CODEOWNERS#L9

github-actions[bot] commented 2 months ago

This issue has been automatically marked as stale because it has not had recent activity :sleeping:

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience :heart: