fix/CICD ~ cleanup and refactor with multiple associated fixes by rivy · Pull Request #4974 · uutils/coreutils

rivy · 2023-06-13T02:47:22Z

The plan is for this commit set to get CI to back to green (✔️).
I'll convert from draft when the set builds/tests cleanly.

rivy · 2023-06-13T03:43:43Z

All checks/tests are now passing.
We can use this as a new "all-green" baseline.

sylvestre · 2023-06-13T06:09:41Z

src/uu/touch/src/touch.rs

-                    nanos += 1_000_000_000;
-                    seconds -= 1;
-                }
+            if let Ok(offset) = parse_datetime::from_str(date) {


please move this change into a different PR

I can, but the point was to get "green" and stop all the failed CI builds.

It doesn't really matter IMHO

sylvestre · 2023-06-13T06:22:38Z

.taplo.toml

@@ -0,0 +1,7 @@
+# `taplo` configuration


i would rather keep the defaults of taplo

It splits many of the dependency lines in the Cargo.toml files making it harder to keep them organized.
And it pushes around end-of-line comments to ridiculous locations.

Do you have examples of that?

For example, this one seems unrelated:
array_auto_collapse = false

I added them iteratively as formatting problems cropped up.
I don't remember the details for the need for that line.
It might have been needed before the final format was completed, and redundant now.
I'll remove that line it locally and see if anything crops back up.
...
I found the issue. Without that configuration, taplo collapses all the feature sets (eg, feat_common_core, feat_Tier1, etc) into single lines, making them harder to read and curate.

Basically, some TOML arrays are more readable/useful collapsed and some are better expanded.
Just collapsing (or expanding) all of them removes the textual clues that help reduce cognitive load when reading through and updating the code.

All of the configuration tags here are purposeful and help me keep the TOML organized and readable.

ok, i don't like it but i won't spend my time arguing about coding style. I just wish you didn't change the default...
anyway, if you want to change them, please do it a separate PR

sylvestre · 2023-06-13T06:23:11Z

deny.toml

  "BSD-3-Clause",
  "CC0-1.0",
-  "MPL-2.0",              # XXX considered copyleft?
+  # "MPL-2.0",              # * unused # ToDO?: considered copyleft? remove?


please just remove it :)

rivy · 2023-06-13T12:41:22Z

I'm rechecking the Android CI step; it looks like it timed-out waiting for the emulator.

sylvestre · 2023-06-13T12:49:49Z

.github/workflows/CICD.yml

-        name: toybox-result.json
-        path: ${{ steps.vars.outputs.TEST_SUMMARY_FILE }}
-
-  toml_format:


please put it back in its own tasks. It makes reviews easier

sylvestre

Could you please split this PR into smaller?
it is a mixing way too many different things in the same PR.

I don't mind having individual PR being red

Thanks

rivy · 2023-06-13T14:02:59Z

Cargo.toml

 textwrap = { version = "0.16.0", features = ["terminal_size"] }
 thiserror = "1.0"
-time = { version = "0.3" }
+time = { version = "=0.3.20" } ## maint: [2023-06-11; rivy] pinned to maintain MinSRV 1.64.0


@sylvestre , this end-of-line comment gets blown out to starting at 100+ character position without the align_comments = false.

you can probably make the comment shorter. remove the "maint" and the "maintain"
or move it the line above

IMO, it's better located on the line that it refers to so that they don't get separated with future change.
Ultimately, it'll be removable (and should be removed along with the = pin) when MinSRV increases.
And, no matter the length, if it's on the same line, taplo is pushing it to the right side, with an extremely wide area of white space between it and the code it refers too.

rivy · 2023-06-13T14:44:05Z

The Android CI tests are consistently failing on the emulator setup.
We should likely just disable it with a "maint" tag to work on it later when someone has the expertise and will to fix it.

rivy · 2023-06-13T14:59:58Z

Could you please split this PR into smaller? it is a mixing way too many different things in the same PR.

I don't mind having individual PR being red

Thanks

Sure, I'll try to refine it and split it.
It's conceptually a whole in that it is fixing the technical debt of allowing multiple failed CI merges.
I'll try to remove any commits that aren't directly contributing to getting CI back into working order.

I get that a failed CI isn't the worst of issues, but it becomes chronic. And then things start slipping through that aren't seen because we think we know why it's failing. It becomes much more messy to fix it.

sylvestre · 2023-09-24T08:41:11Z

most of the changes have been merged: closing

rivy changed the title ~~CI refactor with multiple fixes~~ CI cleanup/refactor with multiple fixes Jun 13, 2023

rivy changed the title ~~CI cleanup/refactor with multiple fixes~~ fix/CICD ~ cleanup and refactor with multiple assocaited fixes Jun 13, 2023

rivy changed the title ~~fix/CICD ~ cleanup and refactor with multiple assocaited fixes~~ fix/CICD ~ cleanup and refactor with multiple associated fixes Jun 13, 2023

rivy marked this pull request as ready for review June 13, 2023 03:44

sylvestre reviewed Jun 13, 2023

View reviewed changes

uutils deleted a comment from github-actions bot Jun 13, 2023

sylvestre reviewed Jun 13, 2023

View reviewed changes

sylvestre requested changes Jun 13, 2023

View reviewed changes

rivy commented Jun 13, 2023

View reviewed changes

uutils deleted a comment from github-actions bot Jun 13, 2023

rivy force-pushed the fixes branch 4 times, most recently from 7f617c6 to b3afd73 Compare June 19, 2023 01:37

rivy added 3 commits June 18, 2023 20:56

maint/CICD ~ refactor for clarity/consistency

34c5873

maint/CICD ~ update build detail display

b0b4363

maint/CICD ~ merge TOML testing into 'Style/format'

0928706

rivy force-pushed the fixes branch 2 times, most recently from 2ccc524 to 907641c Compare June 19, 2023 02:05

rivy added 5 commits June 18, 2023 21:15

maint/CICD ~ recategorize/reorganize CICD main steps

8f76120

docs ~ (cSpell) update dictionary words (people)

b3d6d3f

deps ~ pin time to v0.3.20 (preserves MinSRV v1.64.0)

9264179

deps ~ change from 'humantime_to_duration' to 'parse_datetime'

2a3a9ed

deps ~ (uucore) refresh deps (and reformat for consistency)

bbd18aa

rivy added 7 commits June 18, 2023 21:15

deps ~ reformat for consistency/comprehension

8de576b

deps ~ update 'Cargo.lock'

848297b

maint/dev ~ update cargo deny configuration

47a1fe1

fix/touch ~ time crate usage errors

0007ede

tests ~ fix (platform-specific) warnings

f31ca9a

maint/dev ~ add taplo configuration

56aa906

refactor ~ taplo fmt

1fd90d4

rivy force-pushed the fixes branch from 907641c to 1fd90d4 Compare June 19, 2023 02:15

uutils deleted a comment from github-actions bot Jun 20, 2023

cakebaker mentioned this pull request Jul 3, 2023

touch: use parse_datetime instead of humantime_to_duration #5030

Merged

cakebaker mentioned this pull request Jul 13, 2023

deny.toml: remove MPL license from allow list #5076

Merged

sylvestre closed this Sep 24, 2023

Uh oh!

Conversation

rivy commented Jun 13, 2023

Uh oh!

rivy commented Jun 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rivy Jun 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rivy Jun 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rivy commented Jun 13, 2023

Uh oh!

sylvestre Jun 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sylvestre left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rivy commented Jun 13, 2023

Uh oh!

rivy commented Jun 13, 2023

Uh oh!

sylvestre commented Sep 24, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rivy commented Jun 13, 2023 •

edited

Loading

rivy Jun 13, 2023 •

edited

Loading

rivy Jun 13, 2023 •

edited

Loading

sylvestre Jun 13, 2023 •

edited

Loading