make date-check more lightweight #1394

tshepang · 2022-07-16T02:01:03Z

No description provided.

JohnTitor

Makes some sense to me.

src/contributing.md

spastorino · 2022-07-18T16:46:36Z

Hmmm, I'm neutral to this change but consider that after this is merged, the text is more likely to be changed or removed to something else without being noticed.

ci/date-check/src/main.rs

camelid

Left a few comments above. Overall this looks good. Nice idea!

camelid · 2022-07-18T16:50:14Z

Hmmm, I'm neutral to this change but consider that after this is merged, the text is more likely to be changed or removed to something else without being noticed.

Hmm, that is a good point. It looking more natural does make it less obvious that it's read by a tool...

rust-lang#1394 (review)

tshepang · 2022-07-18T20:37:13Z

I found it tedious to write the date twice when updating things. I also made it more clear what date formats are accepted (as suggested by @camelid), so hopefully it's less hidden if the formats are processed by a tool. Am also thinking that as a WG, we are all aware of this tool (at least the more active of us), and all PRs that violate the format can be fixed, without even burdening the submitter to comply, since we could just add a fixup commit to their PR.

spastorino · 2022-07-19T14:11:37Z

If we don't want to repeat the date and still be explicit that we are checking it, what about ...

When you don't want the date to show up in the text: 
When you want the date to show up in the text: July 2022

camelid · 2022-07-24T03:45:34Z

If we don't want to repeat the date and still be explicit that we are checking it, what about ...

When you don't want the date to show up in the text:  When you want the date to show up in the text: July 2022

I like this idea. Or even shorter:  and July 2022. Maybe that wouldn't be clear enough though?

tshepang · 2022-07-24T03:48:19Z

I named it date-check in a branch am still working on, which matches name of tool

This avoids having to write the date twice when updating date-check. Before "As of <-- 2022-07 --> July 2022" After "As of July 2022"

Co-authored-by: Noah Lev <[email protected]>

rust-lang#1394 (review)

Co-authored-by: Noah Lev <[email protected]>

rust-lang#1394 (review)

rust-lang#1394 (comment)

tshepang · 2022-07-30T22:13:53Z

This was rebased, but the only real change is the latest commit e82c245.

camelid

I think this is very close. My only main concern is the complexity of having two regexes. I think it should be possible to have just one (slightly more permissive) regex. The rest of the comments are just stylistic nitpicks.

camelid · 2022-07-31T17:40:06Z

ci/date-check/src/main.rs

+        r"<!--\s+date-check:\s+(\w+)\s+(\d+{4})\s+-->",
+        r"<!--\s+date-check\s+-->\s+(\w+)\s+(\d+{4})",


I don't think the + after \d is correct (it wasn't there before either):

Suggested change

r"",

r"\s+(\w+)\s+(\d+{4})",

r"",

r"\s+(\w+)\s+(\d{4})",

Actually, could you combine these regexes into one to avoid all this complexity with making multiple passes? It might be a little more permissive wrt accepted input but I think it'd be better overall.

I did this with an earlier iteration, but it moved complexity elsewhere, where I had 4 regex groups (instead of just 2). Is that preferable, or is there a better alternative?

ok, it's not so bad 2a5b586

camelid · 2022-07-31T17:41:28Z

ci/date-check/src/main.rs

+    let set = RegexSet::new(&patterns).unwrap();
+    set.patterns()
+        .iter()
+        .map(|pattern| Regex::new(pattern).unwrap())
+        .collect()


What does this usage of RegexSet do? It seems like has the same result as just returning vec![Regex::new(r1), Regex::new(r2)].

a confused mess is what it is

ci/date-check/src/main.rs

src/contributing.md

camelid · 2022-07-31T17:51:11Z

src/contributing.md

+    For the action to pick the date, add this annotation:
+
+        <!-- date-check -->
+
+    Example:
+
+        As of <!-- date-check --> Jul 2022, the foo did the bar.
+
+    For cases where the date should not be part of the visible rendered output,
+    use the following instead:
+
+        <!-- date-check: Jul 2022 -->


nit: could you use fenced code blocks and mark it as markdown?

rust-lang#1394 (comment) This led to a more robust regex, though making the tool more picky. It also found a wrong date format that was missed.

rust-lang#1394 (comment)

This was reverted by mistake Co-authored-by: Noah Lev <[email protected]>

rust-lang#1394 (comment)

rust-lang#1394 (comment) Much more clean

tshepang · 2022-07-31T21:35:54Z

@camelid much wonderful review, thanks much... things are better as a result

camelid

Ok, I left some suggestions to simplify the regex. After that, this looks great! Thanks for implementing this and writing so many tests.

ci/date-check/src/main.rs

Co-authored-by: Noah Lev <[email protected]>

Also, test new shape

New regex was introduced by 456008c

camelid

Thank you again! I really like this new version of date-check.

Update books ## nomicon 2 commits in 8d1e4dccf71114ff56f328f671f2026d8e6b62a2..8e6aa3448515a0654e347b5e2510f1d4bc4d5a64 2022-07-18 18:12:35 -0400 to 2022-08-15 15:36:13 -0700 - Update the `repr(transparent)` section to reflect the current state (rust-lang/nomicon#376) - [typo] typo on limits of lifetime chapter (rust-lang/nomicon#377) ## reference 5 commits in f3d3953bf3b158d596c96d55ce5366f9f3f972e9..e647eb102890e8927f488bea12672b079eff8d9d 2022-08-01 17:17:37 -0700 to 2022-08-16 11:35:27 -0700 - #[non_exhaustive] on variant blocks cross-crate as casts (rust-lang/reference#1249) - Revert let chains reference docs (rust-lang/reference#1251) - Update subtyping.md (rust-lang/reference#1240) - a fix about .await (rust-lang/reference#1245) - Be less specific about the representation of `+bundle` (rust-lang/reference#1246) ## book 2 commits in 36383b4da21dbd0a0781473bc8ad7ef0ed1b6751..42ca0ef484fcc8437a0682cee23abe4b7c407d52 2022-07-19 21:03:20 -0400 to 2022-08-12 21:52:02 -0400 - Missing period at end of sentence - Fix grammar in ch06-02 ## rust-by-example 8 commits in ee342dc91e1ba1bb1e1f1318f84bbe3bfac04798..03301f8ae55fa6f20f7ea152a517598e6db2cdb7 2022-07-27 11:06:36 -0300 to 2022-08-14 08:51:44 -0300 - Update print.md (rust-lang/rust-by-example#1597) - in Meta, replace 'playpen' with 'playground' (rust-lang/rust-by-example#1596) - Update doc comment to link to name field without compilation warning (rust-lang/rust-by-example#1595) - add line numbers for playpen fixes rust-lang/rust-by-example#1593 (rust-lang/rust-by-example#1594) - clarify that the map-reduce example relies on static data (rust-lang/rust-by-example#1592) - Update flow_control.md (rust-lang/rust-by-example#1591) - Remove duplicate line in the hello/print.md file (rust-lang/rust-by-example#1590) - Make rust editable in chapter on defaults (rust-lang/rust-by-example#1589) ## rustc-dev-guide 15 commits in 04f3cf0..d3daa1f 2022-07-31 07:46:57 +0200 to 2022-08-13 10:00:38 +0900 - Improve the "Diagnostic items" chapter (rust-lang/rustc-dev-guide#1427) - date-check: crates-io - fix/improve compiler-debugging - Update src/compiler-debugging.md - add gdb tips for symbol-mangling-version - move references down to avoid clutter (rust-lang/rustc-dev-guide#1420) - update date-check format on github issue (rust-lang/rustc-dev-guide#1416) - Fix legend colors in dark mode - Add color for downloaded nodes - Add colors to diagram - Add bootstrapping diagram - date-check: rustc_codegen_ssa is still alive - note is now too old to be relevant - date-check: be more strict - make date-check more lightweight (rust-lang/rustc-dev-guide#1394) ## edition-guide 3 commits in c55611dd6c58bdeb52423b5c52fd0f3c93615ba8..6038be9d37d7251c966b486154af621d1794d7af 2022-02-21 14:21:39 +0100 to 2022-08-15 08:12:42 -0700 - use title "The Rust Edition Guide" everywhere (rust-lang/edition-guide#280) - "Creating a new project": add example using 'cargo new --edition YEAR' (rust-lang/edition-guide#279) - fixes rust-lang/edition-guide#277: mention rust 1.0 release month and year (rust-lang/edition-guide#278)

JohnTitor approved these changes Jul 16, 2022

View reviewed changes

camelid self-assigned this Jul 17, 2022

camelid added the S-waiting-on-review Status: this PR is waiting for a reviewer to verify its content label Jul 17, 2022

camelid reviewed Jul 18, 2022

View reviewed changes

src/contributing.md Outdated Show resolved Hide resolved

camelid reviewed Jul 18, 2022

View reviewed changes

src/contributing.md Show resolved Hide resolved

camelid reviewed Jul 18, 2022

View reviewed changes

ci/date-check/src/main.rs Outdated Show resolved Hide resolved

camelid reviewed Jul 18, 2022

View reviewed changes

ci/date-check/src/main.rs Outdated Show resolved Hide resolved

camelid requested changes Jul 18, 2022

View reviewed changes

tshepang added a commit to tshepang/rustc-dev-guide that referenced this pull request Jul 18, 2022

address review comment

498f62b

rust-lang#1394 (review)

tshepang added a commit to tshepang/rustc-dev-guide that referenced this pull request Jul 18, 2022

address review comment

c4f7106

rust-lang#1394 (review)

tshepang requested a review from camelid July 18, 2022 20:27

tshepang and others added 8 commits July 30, 2022 21:47

make date-check lightweight

b570c88

This avoids having to write the date twice when updating date-check. Before "As of <-- 2022-07 --> July 2022" After "As of July 2022"

please clippy

5b5ab38

update date-check docs

5524470

accept review suggestion

d946154

Co-authored-by: Noah Lev <[email protected]>

address review comment

9eef34f

rust-lang#1394 (review)

accept review suggestion

c9fcae3

Co-authored-by: Noah Lev <[email protected]>

address review comment

f4803a7

rust-lang#1394 (review)

address review comment

e82c245

rust-lang#1394 (comment)

tshepang force-pushed the lightweight branch from c4f7106 to e82c245 Compare July 30, 2022 22:10

camelid reviewed Jul 31, 2022

View reviewed changes

camelid added S-waiting-on-author Status: this PR is waiting for additional action by the OP and removed S-waiting-on-review Status: this PR is waiting for a reviewer to verify its content labels Jul 31, 2022

tshepang and others added 9 commits July 31, 2022 21:00

this breaks markdown

df6b805

address review comment

3faf82c

rust-lang#1394 (comment) This led to a more robust regex, though making the tool more picky. It also found a wrong date format that was missed.

address review comment

74b9b45

rust-lang#1394 (comment)

address review comment

ea2f1c2

rust-lang#1394 (comment)

accept review suggestion

e697cb6

This was reverted by mistake Co-authored-by: Noah Lev <[email protected]>

address review comment

dcfb973

rust-lang#1394 (comment)

use a more simple fn

ade2924

address review comment

2a5b586

rust-lang#1394 (comment) Much more clean

nit

483a0a5

camelid reviewed Aug 1, 2022

View reviewed changes

ci/date-check/src/main.rs Outdated Show resolved Hide resolved

ci/date-check/src/main.rs Outdated Show resolved Hide resolved

tshepang and others added 3 commits August 2, 2022 05:38

accept review suggestion

456008c

Co-authored-by: Noah Lev <[email protected]>

avoid a failed regex

c60a888

Also, test new shape

adjust to new regex (which uses named groups)

86ecc94

New regex was introduced by 456008c

camelid approved these changes Aug 2, 2022

View reviewed changes

camelid merged commit 2557089 into rust-lang:master Aug 2, 2022

tshepang deleted the lightweight branch August 2, 2022 21:04

ehuss mentioned this pull request Aug 17, 2022

Update books rust-lang/rust#100655

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make date-check more lightweight #1394

make date-check more lightweight #1394

tshepang commented Jul 16, 2022

JohnTitor left a comment

spastorino commented Jul 18, 2022

camelid left a comment

camelid commented Jul 18, 2022

tshepang commented Jul 18, 2022 •

edited

Loading

spastorino commented Jul 19, 2022

camelid commented Jul 24, 2022

tshepang commented Jul 24, 2022

tshepang commented Jul 30, 2022

camelid left a comment

camelid Jul 31, 2022

camelid Jul 31, 2022

tshepang Jul 31, 2022

tshepang Jul 31, 2022

camelid Jul 31, 2022

tshepang Jul 31, 2022

camelid Jul 31, 2022

tshepang commented Jul 31, 2022

camelid left a comment

camelid left a comment

		r"<!--\s+date-check:\s+(\w+)\s+(\d+{4})\s+-->",
		r"<!--\s+date-check\s+-->\s+(\w+)\s+(\d+{4})",

make date-check more lightweight #1394

make date-check more lightweight #1394

Conversation

tshepang commented Jul 16, 2022

JohnTitor left a comment

Choose a reason for hiding this comment

spastorino commented Jul 18, 2022

camelid left a comment

Choose a reason for hiding this comment

camelid commented Jul 18, 2022

tshepang commented Jul 18, 2022 • edited Loading

spastorino commented Jul 19, 2022

camelid commented Jul 24, 2022

tshepang commented Jul 24, 2022

tshepang commented Jul 30, 2022

camelid left a comment

Choose a reason for hiding this comment

camelid Jul 31, 2022

Choose a reason for hiding this comment

camelid Jul 31, 2022

Choose a reason for hiding this comment

tshepang Jul 31, 2022

Choose a reason for hiding this comment

tshepang Jul 31, 2022

Choose a reason for hiding this comment

camelid Jul 31, 2022

Choose a reason for hiding this comment

tshepang Jul 31, 2022

Choose a reason for hiding this comment

camelid Jul 31, 2022

Choose a reason for hiding this comment

tshepang commented Jul 31, 2022

camelid left a comment

Choose a reason for hiding this comment

camelid left a comment

Choose a reason for hiding this comment

tshepang commented Jul 18, 2022 •

edited

Loading