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

Update included block example based on ones either side #348

Merged
merged 3 commits into from
Apr 17, 2020
Merged

Conversation

azaroth42
Copy link
Contributor

@azaroth42 azaroth42 commented Apr 10, 2020

Closes #255 ... finally!


💥 Error: 500 Internal Server Error 💥

PR Preview failed to build. (Last tried on Apr 14, 2020, 6:07 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 Spec Generator - Spec Generator is the web service used to build specs that rely on ReSpec.

🔗 Related URL

Navigation timeout of 30000 ms exceeded

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

index.html Outdated
"classification": {"@type": "@vocab"},
"service": {"@type": "@vocab"}
"team": {"@type": "@vocab"}
Copy link
Member

Choose a reason for hiding this comment

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

Does this statement (and the previous one) add anything to the example? Ie, is it necessary to have it here? At first glance I do not think so; if that is correct, it is better to remove it as just blurring the essence of the example.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we could settle on either "classification" or "team", don't need both.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I was wondering about "@type":"@value", not the fact of having both...

"http://example.org/classification": [
{"@id": "http://example.org/enum#c6"}
{"@id": "http://example.org/employee"}
Copy link
Member

Choose a reason for hiding this comment

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

The statement on the name of Manu is missing...

Copy link
Member

Choose a reason for hiding this comment

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

That could be told by running the "rake" task, which is also noted by travis. It checks that the first example matches the things in the second example after running through the algorithms.

bundle install
rake

That's all that should be necessary.

"http://example.org/classification": [
{"@id": "http://example.org/enum#c6"}
{"@id": "http://example.org/contractor"}
Copy link
Member

Choose a reason for hiding this comment

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

The statement on the name of Gregg is missing...

Copy link
Member

@gkellogg gkellogg left a comment

Choose a reason for hiding this comment

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

Consolidate classification and team, and make sure that the rake task succeeds.

Or, ask me to finish it.

index.html Outdated
"classification": {"@type": "@vocab"},
"service": {"@type": "@vocab"}
"team": {"@type": "@vocab"}
Copy link
Member

Choose a reason for hiding this comment

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

Yes, we could settle on either "classification" or "team", don't need both.

"http://example.org/classification": [
{"@id": "http://example.org/enum#c6"}
{"@id": "http://example.org/employee"}
Copy link
Member

Choose a reason for hiding this comment

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

That could be told by running the "rake" task, which is also noted by travis. It checks that the first example matches the things in the second example after running through the algorithms.

bundle install
rake

That's all that should be necessary.

@azaroth42
Copy link
Contributor Author

I wanted to demonstrate with the two different properties that it's not just a single relationship, it's a grab bag of any entities that you want to reference. If we don't think that's an issue, then yes, we can remove team.

@iherman
Copy link
Member

iherman commented Apr 13, 2020

@azaroth42 see #348 (comment)

@gkellogg
Copy link
Member

I wanted to demonstrate with the two different properties that it's not just a single relationship, it's a grab bag of any entities that you want to reference. If we don't think that's an issue, then yes, we can remove team.

I don't see how this relates to included blocks; that's certainly what has been highlighted.

@azaroth42
Copy link
Contributor Author

@BigBlueHat @pchampin ?

@gkellogg gkellogg merged commit a062b86 into master Apr 17, 2020
@gkellogg gkellogg deleted the 255_finally branch April 17, 2020 16:06
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.

Better example for included blocks
5 participants