Skip to content
This repository was archived by the owner on Feb 3, 2023. It is now read-only.

Cause hc-test-fmt to error if any errors #1713

Closed
wants to merge 5 commits into from
Closed

Conversation

maackle
Copy link
Collaborator

@maackle maackle commented Sep 17, 2019

PR summary

hc-test-fmt stopped failing. This makes it fail when there are fmt diffs.

However hn-rust-fmt is also broken, due to this error:

thread 'main' panicked at 'bad span: `.`: `)`', src/tools/rustfmt/src/source_map.rs:52:13

Which is referenced here: rust-lang/rustfmt#3666. So, we have no way to fix the fmt errors that are now being reported by CI

changelog

Please check one of the following, relating to the CHANGELOG-UNRELEASED.md
thread 'main' panicked at 'bad span: .: )', src/tools/rustfmt/src/source_map.rs:52:13

  • this is a code change that effects some consumer (e.g. zome developers) of holochain core so it is added to the CHANGELOG-UNRELEASED.md (linked above), with the format - summary of change [PR#1234](https://github.com/holochain/holochain-rust/pull/1234)
  • this is not a code change, or doesn't effect anyone outside holochain core development

Copy link
Contributor

@thedavidmeister thedavidmeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assuming this is just the pipefail and everything else is noise due to the merge conflict, i'm happy @maackle

@maackle
Copy link
Collaborator Author

maackle commented Oct 1, 2019

@thedavidmeister everything is was a manual rustfmt run, because the holonix fmt fix isn't working, and we already have a bunch of code that fails the fmt check

@thedavidmeister
Copy link
Contributor

@maackle oh sure, that makes sense, i still approve

@maackle
Copy link
Collaborator Author

maackle commented Nov 4, 2019

Fixed in #1822

@maackle maackle closed this Nov 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants