Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upimplement valid week string #19559
implement valid week string #19559
Conversation
highfive
commented
Dec 14, 2017
|
Heads up! This PR modifies the following files:
|
| @@ -30,6 +30,9 @@ | |||
| [[week\] stepDown method support on input 'week' element] | |||
| expected: FAIL | |||
|
|
|||
| [[week\] The value must be a value that is a valid global date and time string] | |||
This comment has been minimized.
This comment has been minimized.
tigercosmos
Dec 14, 2017
Author
Collaborator
not caused by this PR, since datetime has not implemented
| @@ -1,20 +1,13 @@ | |||
| [week.html] | |||
| type: testharness | |||
| [2014 has 52 weeks: Value should be empty] | |||
| [Value >= min attribute] | |||
This comment has been minimized.
This comment has been minimized.
| @@ -358,6 +358,15 @@ dependencies = [ | |||
| "libc 0.2.33 (registry+https://github.com/rust-lang/crates.io-index)", | |||
| ] | |||
|
|
|||
| [[package]] | |||
| name = "chrono" | |||
This comment has been minimized.
This comment has been minimized.
|
Just a couple of small nits! r=me after you make those changes. |
| } | ||
|
|
||
| fn is_leap_year(year: u32) -> bool { | ||
| if year % 400 == 0 || (year % 4 == 0 && year % 100 != 0) { |
This comment has been minimized.
This comment has been minimized.
KiChjang
Dec 15, 2017
Member
The if here is unnecessary, you can just return year % 400 == 0 || (year % 4 == 0 && year % 100 != 0).
| } | ||
| } | ||
|
|
||
| fn is_leap_year(year: u32) -> bool { |
This comment has been minimized.
This comment has been minimized.
| fn max_week_in_year(year: u32) -> u32 { | ||
| match Utc.ymd(year as i32, 1, 1).weekday() { | ||
| Weekday::Thu => 53, | ||
| Weekday::Wed => { |
This comment has been minimized.
This comment has been minimized.
| } | ||
|
|
||
| // Step 5, 6 | ||
| let week = iterator.next().ok_or(())?; |
This comment has been minimized.
This comment has been minimized.
KiChjang
Dec 15, 2017
Member
We also need to ensure that there are no more items left to iterate on the iterator after this line. This is in fact step 10 of the spec.
This comment has been minimized.
This comment has been minimized.
|
@KiChjang Please check. |
| } | ||
|
|
||
| // Step 10 | ||
| if iterator.next().is_some() || &value[(value.len() - 1)..] == "-" { |
This comment has been minimized.
This comment has been minimized.
KiChjang
Dec 17, 2017
Member
The last character match here is not necessary, since split produces an empty string for each - it encounters. https://play.rust-lang.org/?gist=e50b834be9123f860f52b5762c2e07d8&version=stable
|
@KiChjang fixed. |
|
@bors-servo r+ Thanks! |
|
|
implement valid week string <!-- Please describe your changes on the following line: --> implement valid week string part of #19172 r? @KiChjang --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [x] There are tests for these changes <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19559) <!-- Reviewable:end -->
|
|
|
@bors-servo retry |
|
|
|
|
Upstreamed from servo/servo#19559 [ci skip]
tigercosmos commentedDec 14, 2017
•
edited
implement valid week string
part of #19172
r? @KiChjang
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is