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

Issue #2520: Add Text to Range of ApplicationDeadline #2607

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

reginafcompton
Copy link

@reginafcompton reginafcompton commented Jun 22, 2020

Description

This PR updates the ApplicationDeadline property to include Text as an expected type.

Resolves issue #2520 .

Notes for the reviewer

  1. This is my first Schema.org PR! I used the issue-nnnn.rdfa-template to get started, but I am unsure if should provide other files, e.g., a ttl file. Do I need to make modifications elsewhere?
  2. I also modified the description in the comment, though it may be unnecessarily verbose now. What do you think?

@reginafcompton reginafcompton marked this pull request as ready for review June 22, 2020 19:55
@RichardWallis
Copy link
Contributor

@reginafcompton Unfortunately your first PR coincided with our move from rdfa to ttl.

A issue-2520.ttl version will need creating in replacement for the issue-2520.rdfa file.

To help, here is what would be the contents:

@prefix rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#> .
@prefix rdfs: <http://www.w3.org/2000/01/rdf-schema#> .

:applicationDeadline a rdf:Property ;
    rdfs:label "applicationDeadline" ;
    :category "issue-2419" ;
    :domainIncludes :EducationalOccupationalProgram ;
    :rangeIncludes :Date,
        :Text ;
    :source <https://github.com/schemaorg/schemaorg/issues/2419> ;
    rdfs:comment "The date at which the program stops collecting applications for the next enrolment cycle. Flexible application deadlines (for example, a program with rolling admissions) can be described in a textual string, rather than as a DateTime. " .

@reginafcompton
Copy link
Author

@RichardWallis - thank you!

I added a ttl file per your instructions. A couple notes:

  • the Ruby linter is failing (for my PR and other recent PRs, e.g., this one) – do you have any thoughts on this?
  • I was not able to amend the comment property of applicationDeadline. I'd like to add a sentence about "rolling admissions," but this results in failing tests. See this commit for full context.

@RichardWallis
Copy link
Contributor

@reginafcompton I have just checked back into this PR - I see you have been having a few issues.

  • The Ruby linter issue was nothing to do with your PR - the problem has been fixed elsewhere and the checks now pass on your PR.
  • The problem with your comment amendment appears to be that it resulted in a multi-line string. In Turtle, double quotes can only encompass a single line of text. Checking out the Turtle Standard regarding literals you will find that if either you should remove the line breaks from within your comment or enclosed it with triple quotes (''').

Hope that helps.

@reginafcompton
Copy link
Author

@RichardWallis - thank you for your notes! I fixed my PR (in part by directly editing an existing tty file within the pending dir, which I previously overlooked).

Please pass along additional feedback, and let me know any next steps I can take.

@RichardWallis RichardWallis changed the base branch from master to main July 23, 2020 13:04
@github-actions
Copy link

This pull request is being tagged as Stale due to inactivity.

@github-actions github-actions bot added the no-pr-activity Discuss has gone quiet. Auto-tagging to encourage people to re-engage with the issue (or close it!). label Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-pr-activity Discuss has gone quiet. Auto-tagging to encourage people to re-engage with the issue (or close it!).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants