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

Added entry for POWDER-S #459

Merged
merged 1 commit into from
Apr 24, 2018
Merged

Added entry for POWDER-S #459

merged 1 commit into from
Apr 24, 2018

Conversation

andrea-perego
Copy link
Collaborator

@andrea-perego andrea-perego merged commit c8929eb into master Apr 24, 2018
@andrea-perego andrea-perego deleted the andrea-perego-patch-1 branch April 24, 2018 22:09
@marcoscaceres
Copy link
Collaborator

I’m a bit uncomfortable that people are able to merge their own pull requests. @tobie, can you change the rules to require one review? For instance, why is this spec not coming from TR automatically? Also, I see this introduced unexpected white space.

@tobie
Copy link
Owner

tobie commented Apr 24, 2018

I’m a bit uncomfortable that people are able to merge their own pull requests. @tobie, can you change the rules to require one review?

So enforcing that is very difficult when you have have a bot.

It also runs contrary to the spirit of the project which trust people to do the right thing.

That said, I agree in this case that this is sort of special (non TR W3C spec?) and might have benefitted from a review. @andrea-perego, think you can be a bit more conservative in the future? Thanks!

For instance, why is this spec not coming from TR automatically?

I don't think this document is on TR.

Also, I see this introduced unexpected white space.

The bot fixes that: ce7b07f.

@andrea-perego
Copy link
Collaborator Author

Sorry, I didn't think I was breaking the rules, and of not being conservative.

The reason why I added this entry is exactly because POWDER-S was not coming directly from the W3C TR entries, although it is referenced from the relevant W3C REC Protocol for Web Description Resources (POWDER): Formal Semantics. If this raises any concern, I'm perfectly happy to remove it from SpecRef.

BTW, no problem for me to ask for a review (who should it be, BTW?).

@andrea-perego
Copy link
Collaborator Author

A side note:

If there's a rule according to which documents in W3C space cannot be added to SpecRef without W3C consent, I think this should be explicitly stated.

If this is actually the case, then probably it should also be applied to those specs from other standard bodies (as ISO and OGC) already included in SpecRef - and, as far as I know, not from an "official" channel.

@marcoscaceres
Copy link
Collaborator

@tobie, wrote

So enforcing that is very difficult when you have have a bot.

Understood. We can look at maybe giving a personalized token to the bot (they can have admin rights allowing them to bypass review)?

It also runs contrary to the spirit of the project which trust people to do the right thing.

I'm sympathetic to this, but the right thing on GitHub is usually to ask for review. It helps to have a least one person have a quick look over things, if only to check for spelling errors, copy/pasta, or confirm URLs are https where possible.

@andrea-perego:

BTW, no problem for me to ask for a review (who should it be, BTW?).

There is a list of folks that appears in the Reviews drop down who are happy to help. If unsure, you can tag me or @tobie.

If there's a rule according to which documents in W3C space cannot be added to SpecRef without W3C consent, I think this should be explicitly stated.

It's not about consent, and there are no such rules. However, if something in TR and it's not being automatically added to SpecRef, then it might be problem upstream.

@andrea-perego
Copy link
Collaborator Author

It's not about consent, and there are no such rules. However, if something in TR and it's not being automatically added to SpecRef, then it might be problem upstream.

Understood. POWDER-S is the namespace document of the RDF vocabulary described in POWDER-FORMAL, but without providing the list of classes and properties. It is not in TR space, so this is probably why it is now included in w3c.json.

@tobie
Copy link
Owner

tobie commented Apr 25, 2018

I'm sympathetic to this, but the right thing on GitHub is usually to ask for review.

GitHub generally handles code, not data entries whose format is enforced by a schema.

Here's part of my comment to people that have gotten their first PR here merged:

Specref loosely follows the process described in The Pull Request Hack. You've thus been granted commit access to the repo.

Please read-up on how to make manual changes, get non-trivial changes reviewed by someone (I'm sure you'll be a good judge of when that's necessary) and feel free to ask if something's unclear.

This has worked very well so far, imho, and I don't remember an incidence of this being an issue for anyone up to today.

Looking at both of @andrea-perego's PR today, it appears that he did the right thing both times, but it wasn't obvious to an external eye.

I think we should continue with the same policy until we have an actual issue (or a number of smaller-sized concerns).

Additionally @andrea-perego, please make sure you document the why of your changes more clearly and ping for folks for a review if anything your doing isn't super straightforward.

It helps to have a least one person have a quick look over things, if only to check for spelling errors, copy/pasta, or confirm URLs are https where possible.

I don't disagree. But I'd rather this be something people feel like they can opt-in to rather than are forced to follow.

@tobie
Copy link
Owner

tobie commented Apr 25, 2018

@marcoscaceres I voluntarily left out the above thoughts from the README as I wanted to favor a 1:1 conversation following the first successfully merged PR.

Maybe it's time now to formalize this a bit more and add it to the README.

Thoughts?

@marcoscaceres
Copy link
Collaborator

Additionally @andrea-perego, please make sure you document the why of your changes more clearly and ping for folks for a review if anything your doing isn't super straightforward.

That would be great.

I don't disagree. But I'd rather this be something people feel like they can opt-in to rather than are forced to follow.

Yes, absolutely. I'm sorry if I came across as a gatekeeper. My intention was more on the side of were are here to help look over things.

Maybe it's time now to formalize this a bit more and add it to the README.

I think that would be good.

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

Successfully merging this pull request may close these issues.

3 participants