Page MenuHomePhabricator

[services] Tests - run cargo fmt
ClosedPublic

Authored by karol on Jun 30 2022, 7:09 AM.
Tags
None
Referenced Files
F2205198: D4413.id14351.diff
Sat, Jul 6, 7:31 PM
F2205196: D4413.id14081.diff
Sat, Jul 6, 7:31 PM
F2205195: D4413.id14078.diff
Sat, Jul 6, 7:31 PM
F2205194: D4413.id14075.diff
Sat, Jul 6, 7:31 PM
F2205193: D4413.id14035.diff
Sat, Jul 6, 7:31 PM
F2205192: D4413.id.diff
Sat, Jul 6, 7:31 PM
F2204512: D4413.diff
Sat, Jul 6, 4:46 PM
F2203858: D4413.diff
Sat, Jul 6, 1:26 PM

Details

Summary

Depends on D4379
Something's wrong, cargo fmt should be fired up in git pre commit hooks, this should be fixed.

Test Plan

-

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jun 30 2022, 8:43 AM
Harbormaster failed remote builds in B10165: Diff 14035!

@varun, any idea why cargo fmt isn't being triggered properly in pre-commit hooks?

Here's a theory... lint-staged requires yarn to set things up. Maybe Rust devs aren't running yarn (or yarn cleaninstall) when setting up their repo?

lint-staged is definitely running when I run git commit. I think it has to do with the code being dead, or perhaps the atypical way we're importing modules rn... I tried arc patch-ing the previous diff in the stack to check what's happening, but it failed:

varun@varuns-MacBook-Pro comm % arc patch D4379
 INFO  Base commit is not in local repository; trying to fetch.
Created and checked out branch arcpatch-D4379.
Created and checked out branch arcpatch-D4161.
Checking patch services/commtest/src/main.rs...
Checking patch services/commtest/rustfmt.toml...
Checking patch services/commtest/build.rs...
Checking patch services/commtest/Cargo.toml...
Checking patch services/commtest/Cargo.lock...
Checking patch services/commtest/.gitignore...
Checking patch scripts/rust_pre_commit.sh...
Checking patch package.json...
Checking patch .lintstagedrc.js...
Applied patch services/commtest/src/main.rs cleanly.
Applied patch services/commtest/rustfmt.toml cleanly.
Applied patch services/commtest/build.rs cleanly.
Applied patch services/commtest/Cargo.toml cleanly.
Applied patch services/commtest/Cargo.lock cleanly.
Applied patch services/commtest/.gitignore cleanly.
Applied patch scripts/rust_pre_commit.sh cleanly.
Applied patch package.json cleanly.
Applied patch .lintstagedrc.js cleanly.
 COMMITTED  Successfully committed patch.

 Cherry Pick Failed!
 Exception 
Command failed with error #1!
COMMAND
git cherry-pick -- arcpatch-D4161

STDOUT
CONFLICT (add/add): Merge conflict in services/commtest/Cargo.toml
Auto-merging services/commtest/Cargo.toml
CONFLICT (add/add): Merge conflict in services/commtest/Cargo.lock
Auto-merging services/commtest/Cargo.lock


STDERR
error: could not apply a58da4505... [services] Tests - Initialize project
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'

(Run with `--trace` for a full exception trace.)

@karol-bisztyga once your stack is easier for me to cherry pick I can resolve the cargo fmt issues

This revision is now accepted and ready to land.Jun 30 2022, 1:47 PM
This revision now requires review to proceed.Jun 30 2022, 1:47 PM
In D4413#125182, @varun wrote:

I tried arc patch-ing the previous diff in the stack to check what's happening, but it failed

Running git pull --all --tags before running arc patch should work

In D4413#125190, @atul wrote:
In D4413#125182, @varun wrote:

I tried arc patch-ing the previous diff in the stack to check what's happening, but it failed

Running git pull --all --tags before running arc patch should work

ah ty

Before landing this, please create a task for the cargo fmt not running inside precommit hooks issue. We should make sure to resolve that.

@varun, you should be unblocked with the command @atul referenced above

This revision is now accepted and ready to land.Jun 30 2022, 1:52 PM

Before landing this, please create a task for the cargo fmt not running inside precommit hooks issue. We should make sure to resolve that.

@varun, you should be unblocked with the command @atul referenced above

fixed in D4423

This revision was automatically updated to reflect the committed changes.