-
Notifications
You must be signed in to change notification settings - Fork 19
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
Clarify designated_period_except definition #136
base: dev
Are you sure you want to change the base?
Conversation
Can your teams take a look at this rewording @jacobmalleau @pierre-bouffort @jiffyclub and make sure that this change does not require anyone to redo their existing code, and that it's not breaking? |
Hello @schnuerle and thanks for pointing it out. |
Looks good to me. |
It also seems fine to me - this is a welcome addition to the spec. |
This does now mean that the |
Co-authored-by: Matt Davis <[email protected]>
You may be right here. If we do rename this field then it will have to wait for a 2.0 major release instead of a 1.1.0 patch. Is there some way to clarify this in the field description instead for now? |
@LaurentG-AMD
Policy A allows parking 9h00-11h00 on Fridays, April 1st to November 30th, but not during snow removal. Policy B prohibits parking at all times. Since Policy A has a lower priority it would take precedence over B during the Friday 9-11 timespan. Also, are you still interested in exceptions to user classes? Maybe we could add it to this PR. |
Both the except additions look good on my end, particular the user class one which should help us minimize the long list of user classes (something @RickNeubauer has brought up before. |
@mplsmitch Sorry I did not see your post in August. Your proposed combination is doing exactly the opposite of what my example needs (which is prohibiting all days except fridays), but I'm not debating that we could achieve it via 2 policies. Rather, my point is that we should aim for a single policy per sign so maintaining the database is easier when regulations evolve. I'm not sure my example was the best to showcase this since it was mixing a permanent exception with an exceptional one and those tend to appear on different signs. Here's a better one: take my sign above and change the mention of "friday" for "friday and saturday". I'm pretty sure if i dig long enough I could find a real exemple of such a sign somewhere in the streets here. We still can't really write it efficiently with the proposed implementation: while in This would now read (changes in bold):
My above exemple (friday + snow removal) would now be entered as:
and 2 days exceptions would become
|
Thanks @LaurentG-AMD @jacobmalleau @jiffyclub @mplsmitch what do you think about the proposed changes by @LaurentG-AMD in the comment? Just to document here, I do think the But now I think this kind of proposed change is not a patch, but would be in a minor release, so 1.1. This is more than just a clarification, but instead adding new functionality (that is not breaking in Laurent's suggestion). Plus we are adding a new user_classes_except field, which is fine to do in a minor release, but not a patch. |
@schnuerle I generally agree with what @LaurentG-AMD proposed. My only additional thought, and this is likely just a preference depending on the city, is that ensuring you can uniquely define every type of sign with a unique policy is not always beneficial when defining policies city wide. |
name: Mitch Vars
title: Clarify designated_period_except definition
CDS Pull Request
Explain pull request
The current definition of
designated_period_except
is unclear in that it only covers values named indesignated_period
. This PR expands the definition to cover all fields in the Time Span. For example, Ifdays_of_week
is['sun']
, the Time Span does not apply on Sundays.Additional info in #117
Is this a breaking change
Impacted Spec
Which API(s) will this pull request impact?
Curbs