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

Auto final fixes #922

Merged
merged 4 commits into from Mar 18, 2016

Conversation

Projects
None yet
6 participants
@W3C-GAO
Contributor

W3C-GAO commented Dec 8, 2015

The following issues are addressed by that pull request:

W3C-GAO and others added some commits Dec 3, 2015

auto.schema.org fixes
1) Description for http://auto.schema.org/emissionsCO2 has been
changed, because  there exists no UN/CEFACT Common Code for g/km.

Before:
"The CO2 emissions in g/km. The property uses a number instead of a
QuantitativeValue, since g/km is the dominant unit of measurement, and
there is no UNCEFACT Common Code for g/km.”

After:
"The CO2 emissions in g/km. When used in combination with a
QuantitativeValue, put "g/km" into the unitText property of that value,
since there is no UN/CEFACT Common Code for "g/km"."

2) The text "This should be considered a pre-final preview release;
final changes may be made after wider community review." has been
removed from the extension home page.

3) "http://auto.schema.org/specialUsage" has been renamed to
"http://auto.schema.org/vehicleSpecialUsage".

4) http://schema.org/CarUsageType and its predefined instances:

	http://schema.org/RentalVehicleUsage
	http://schema.org/DrivingSchoolVehicleUsage
	http://schema.org/TaxiVehicleUsage

 have been moved from schema.org core to auto.schema.org.
@mfhepp

This comment has been minimized.

Show comment
Hide comment
@mfhepp

mfhepp Dec 8, 2015

Contributor

Thanks - a few questions:

  1. Is this version somewhere online on a demo site (on appspot)?
  2. From the diff, I think that vehicleSpecialUsage lacks a comment (not 100 % sure).
  3. Did you double-check the examples for mentions of the elements removed from core or renamed in the extension?
Contributor

mfhepp commented Dec 8, 2015

Thanks - a few questions:

  1. Is this version somewhere online on a demo site (on appspot)?
  2. From the diff, I think that vehicleSpecialUsage lacks a comment (not 100 % sure).
  3. Did you double-check the examples for mentions of the elements removed from core or renamed in the extension?
@W3C-GAO

This comment has been minimized.

Show comment
Hide comment
@W3C-GAO

W3C-GAO Dec 8, 2015

Contributor

@mfhepp

  1. Is this version somewhere online on a demo site (on appspot)?
  2. From the diff, I think that vehicleSpecialUsage lacks a comment (not 100 % sure).
  3. Did you double-check the examples for mentions of the elements removed from core or renamed in the extension?

Ad 1. No
Ad 2. vehicleSpecialUsage in the core has a comment. vehicleSpecialUsage in auto extension does not have a comment.
Ad 3. Yes

Contributor

W3C-GAO commented Dec 8, 2015

@mfhepp

  1. Is this version somewhere online on a demo site (on appspot)?
  2. From the diff, I think that vehicleSpecialUsage lacks a comment (not 100 % sure).
  3. Did you double-check the examples for mentions of the elements removed from core or renamed in the extension?

Ad 1. No
Ad 2. vehicleSpecialUsage in the core has a comment. vehicleSpecialUsage in auto extension does not have a comment.
Ad 3. Yes

@mfhepp

This comment has been minimized.

Show comment
Hide comment
@mfhepp

mfhepp Dec 8, 2015

Contributor

@RichardWallis @danbri I think this extension is the first to modify the range of a property from core; can you have a look whether this is fine?

If I understand correctly, the proposal is to have just Text as the range in core and Text OR http://auto.schema.org/CarUsageType.

It might be simpler to move vehicleSpecialUsage to the extension completely and removing it from core.

A counter argument is that in the past, we wanted all properties that are useful for typical car listing sites in the core and externalize only those relevant mostly for manufacturer pages.

Contributor

mfhepp commented Dec 8, 2015

@RichardWallis @danbri I think this extension is the first to modify the range of a property from core; can you have a look whether this is fine?

If I understand correctly, the proposal is to have just Text as the range in core and Text OR http://auto.schema.org/CarUsageType.

It might be simpler to move vehicleSpecialUsage to the extension completely and removing it from core.

A counter argument is that in the past, we wanted all properties that are useful for typical car listing sites in the core and externalize only those relevant mostly for manufacturer pages.

@mfhepp

This comment has been minimized.

Show comment
Hide comment
@mfhepp

mfhepp Dec 8, 2015

Contributor

@W3C-GAO It would be nice to have an appspot version available for review. It should not be a big issue to set up and will be much easier for reviewers than looking at the pure code.

Contributor

mfhepp commented Dec 8, 2015

@W3C-GAO It would be nice to have an appspot version available for review. It should not be a big issue to set up and will be much easier for reviewers than looking at the pure code.

@RichardWallis

This comment has been minimized.

Show comment
Hide comment
@RichardWallis

RichardWallis Dec 8, 2015

Contributor

If specialUsage is defined in the core with a range of Text, you should
only need to define a range of CarUsageType in its definition in the
extension to achieve that objective. The site will sum them together at
run time.

~Richard

On Tue, Dec 8, 2015 at 5:33 PM, Martin Hepp notifications@github.com
wrote:

@RichardWallis https://github.com/RichardWallis @danbri
https://github.com/danbri I think this extension is the first to modify
the range of a property from core; can you have a look whether this is
fine?

If I understand correctly, the proposal is to have just Text as the range
in core and Text OR http://auto.schema.org/CarUsageType.

It might be simpler to move vehicleSpecialUsage to the extension
completely and removing it from core.

A counter argument is that in the past, we wanted all properties that are
useful for typical car listing sites in the core and externalize only those
relevant mostly for manufacturer pages.


Reply to this email directly or view it on GitHub
#922 (comment).

Contributor

RichardWallis commented Dec 8, 2015

If specialUsage is defined in the core with a range of Text, you should
only need to define a range of CarUsageType in its definition in the
extension to achieve that objective. The site will sum them together at
run time.

~Richard

On Tue, Dec 8, 2015 at 5:33 PM, Martin Hepp notifications@github.com
wrote:

@RichardWallis https://github.com/RichardWallis @danbri
https://github.com/danbri I think this extension is the first to modify
the range of a property from core; can you have a look whether this is
fine?

If I understand correctly, the proposal is to have just Text as the range
in core and Text OR http://auto.schema.org/CarUsageType.

It might be simpler to move vehicleSpecialUsage to the extension
completely and removing it from core.

A counter argument is that in the past, we wanted all properties that are
useful for typical car listing sites in the core and externalize only those
relevant mostly for manufacturer pages.


Reply to this email directly or view it on GitHub
#922 (comment).

@trypuz

This comment has been minimized.

Show comment
Hide comment
@trypuz

trypuz Dec 8, 2015

Contributor

@RichardWallis, there is no specialUsage in the core (we had it in the auto extension before but it was changed to vehicleSpecialUsage). So now, we have vehicleSpecialUsage both in the core and in the extension. vehicleSpecialUsage in the core has just Text as the range (i.e. we propose this to be the case); in the extension we only added a range of CarUsageType in its definition.

Contributor

trypuz commented Dec 8, 2015

@RichardWallis, there is no specialUsage in the core (we had it in the auto extension before but it was changed to vehicleSpecialUsage). So now, we have vehicleSpecialUsage both in the core and in the extension. vehicleSpecialUsage in the core has just Text as the range (i.e. we propose this to be the case); in the extension we only added a range of CarUsageType in its definition.

@RichardWallis

This comment has been minimized.

Show comment
Hide comment
@RichardWallis

RichardWallis Dec 8, 2015

Contributor

OK, sorry I was looking at the pre name change version.

Principle is the same however.

If vehicleSpecialUsage is defined as in the core with a range of Text, you
should only need to define a range of CarUsageType in its definition in the
extension to achieve that objective. The site will sum them together at
run time.

Which as you say is what is in pull request #922
#922

~Richard.

On Tue, Dec 8, 2015 at 8:30 PM, trypuz notifications@github.com wrote:

@RichardWallis https://github.com/RichardWallis, there is no
specialUsage in the core (we had it in the auto extension before but it was
changed to vehicleSpecialUsage). So now, we have vehicleSpecialUsage both
in the core and in the extension. vehicleSpecialUsage in the core has just
Text as the range; in the extension we only added a range of CarUsageType
in its definition.


Reply to this email directly or view it on GitHub
#922 (comment).

Contributor

RichardWallis commented Dec 8, 2015

OK, sorry I was looking at the pre name change version.

Principle is the same however.

If vehicleSpecialUsage is defined as in the core with a range of Text, you
should only need to define a range of CarUsageType in its definition in the
extension to achieve that objective. The site will sum them together at
run time.

Which as you say is what is in pull request #922
#922

~Richard.

On Tue, Dec 8, 2015 at 8:30 PM, trypuz notifications@github.com wrote:

@RichardWallis https://github.com/RichardWallis, there is no
specialUsage in the core (we had it in the auto extension before but it was
changed to vehicleSpecialUsage). So now, we have vehicleSpecialUsage both
in the core and in the extension. vehicleSpecialUsage in the core has just
Text as the range; in the extension we only added a range of CarUsageType
in its definition.


Reply to this email directly or view it on GitHub
#922 (comment).

@trypuz

This comment has been minimized.

Show comment
Hide comment
@trypuz

trypuz Dec 17, 2015

Contributor

We created a demo site on appspot of this version of auto extension. Here it is: http://sdo-gao.appspot.com

Contributor

trypuz commented Dec 17, 2015

We created a demo site on appspot of this version of auto extension. Here it is: http://sdo-gao.appspot.com

danbri added a commit that referenced this pull request Mar 18, 2016

@danbri danbri merged commit e7d463a into schemaorg:sdo-deimos Mar 18, 2016

@danbri

This comment has been minimized.

Show comment
Hide comment
@danbri

danbri Mar 18, 2016

Contributor

Note that this update wipes out specialUsage; normally we leave the old definition in place and indicate the newer with 'supersededBy'. In this case, since it was in the stated 'preview' release of the extension, I think we can get away with it as usage levels of specialUsage are likely very low. If it turns out to have been used significantly anywhere we can always add the old name back into the schemas for the record.

Contributor

danbri commented Mar 18, 2016

Note that this update wipes out specialUsage; normally we leave the old definition in place and indicate the newer with 'supersededBy'. In this case, since it was in the stated 'preview' release of the extension, I think we can get away with it as usage levels of specialUsage are likely very low. If it turns out to have been used significantly anywhere we can always add the old name back into the schemas for the record.

@mfhepp

This comment has been minimized.

Show comment
Hide comment
@mfhepp

mfhepp Mar 18, 2016

Contributor

Ooops - I did not remember that we agreed to remove specialUsage. Can anybody point me to the discussion? For rental cars, it is a pretty important information (and mandatory in many countries). Didn't we probably agree to move it to Product?


martin hepp http://www.heppnetz.de
mhepp@computer.org @mfhepp

On 18 Mar 2016, at 15:41, Dan Brickley notifications@github.com wrote:

Note that this update wipes out specialUsage; normally we leave the old definition in place and indicate the newer with 'supersededBy'. In this case, since it was in the stated 'preview' release of the extension, I think we can get away with it as usage levels of specialUsage are likely very low. If it turns out to have been used significantly anywhere we can always add the old name back into the schemas for the record.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

Contributor

mfhepp commented Mar 18, 2016

Ooops - I did not remember that we agreed to remove specialUsage. Can anybody point me to the discussion? For rental cars, it is a pretty important information (and mandatory in many countries). Didn't we probably agree to move it to Product?


martin hepp http://www.heppnetz.de
mhepp@computer.org @mfhepp

On 18 Mar 2016, at 15:41, Dan Brickley notifications@github.com wrote:

Note that this update wipes out specialUsage; normally we leave the old definition in place and indicate the newer with 'supersededBy'. In this case, since it was in the stated 'preview' release of the extension, I think we can get away with it as usage levels of specialUsage are likely very low. If it turns out to have been used significantly anywhere we can always add the old name back into the schemas for the record.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@sdml

This comment has been minimized.

Show comment
Hide comment
@sdml

sdml Mar 18, 2016

@mfhepp We changed the name of specialUsage to vehicleSpecialUsage. See #705 and #898.

sdml commented Mar 18, 2016

@mfhepp We changed the name of specialUsage to vehicleSpecialUsage. See #705 and #898.

@danbri

This comment has been minimized.

Show comment
Hide comment
@danbri

danbri Mar 19, 2016

Contributor

Yes, just a name change - sorry should have been clearer

Contributor

danbri commented Mar 19, 2016

Yes, just a name change - sorry should have been clearer

@danbri

This comment has been minimized.

Show comment
Hide comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment