Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore/reorder imports #3556

Merged
merged 3 commits into from
Feb 28, 2023
Merged

Chore/reorder imports #3556

merged 3 commits into from
Feb 28, 2023

Conversation

pavitthrap
Copy link
Contributor

@pavitthrap pavitthrap commented Feb 7, 2023

Description

  • This PR standardizes the import order for all files such that they are in a maximum of three groups:
  1. A group with std/alloc/core imports
  2. A group with external crate imports
  3. A group for internal imports: self, super, and crate.
  • It updates the CI and checks for the correct import ordering + grouping.
  • It updates the location of CONTRIBUTING.md file (contents in CONTRIBUTORS.md were moved to docs/CONTRIBUTING.md).

I used a rustfmt flag in order to do this reformatting. Here is the command: cargo fmt --all -- --config group_imports=StdExternalCrate. To run just the check, you can run cargo fmt --all -- --check --config group_imports=StdExternalCrate.

Applicable issues

Additional info (benefits, drawbacks, caveats)

Checklist

  • Test coverage for new or modified code paths
  • Changelog is updated
  • Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
  • New clarity functions have corresponding PR in clarity-benchmarking repo
  • New integration test(s) added to bitcoin-tests.yml

@codecov
Copy link

codecov bot commented Feb 7, 2023

Codecov Report

Merging #3556 (5ce2b54) into next (718a352) will increase coverage by 2.08%.
The diff coverage is 30.43%.

@@            Coverage Diff             @@
##             next    #3556      +/-   ##
==========================================
+ Coverage   27.28%   29.36%   +2.08%     
==========================================
  Files         305      309       +4     
  Lines      276796   278266    +1470     
==========================================
+ Hits        75513    81702    +6189     
+ Misses     201283   196564    -4719     
Impacted Files Coverage Δ
clarity/src/libclarity.rs 35.71% <ø> (ø)
clarity/src/vm/analysis/analysis_db.rs 89.28% <ø> (ø)
clarity/src/vm/analysis/arithmetic_checker/mod.rs 75.91% <ø> (+0.72%) ⬆️
...larity/src/vm/analysis/arithmetic_checker/tests.rs 0.00% <ø> (ø)
.../src/vm/analysis/contract_interface_builder/mod.rs 86.47% <ø> (ø)
clarity/src/vm/analysis/errors.rs 26.16% <ø> (-0.47%) ⬇️
clarity/src/vm/analysis/mod.rs 37.00% <ø> (-39.00%) ⬇️
clarity/src/vm/analysis/read_only_checker/mod.rs 77.53% <ø> (-2.65%) ⬇️
clarity/src/vm/analysis/read_only_checker/tests.rs 0.00% <ø> (ø)
clarity/src/vm/analysis/trait_checker/tests.rs 0.00% <ø> (ø)
... and 372 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pavitthrap pavitthrap marked this pull request as ready for review February 8, 2023 18:52
@jcnelson jcnelson added the L1 Working Group Issue or PR related to improving L1 label Feb 21, 2023
@jcnelson jcnelson requested review from jcnelson, kantai and obycode and removed request for jcnelson, kantai and obycode February 28, 2023 16:41
@jcnelson
Copy link
Member

Looks like there needs to be at least one more rebase :/

add more guidance to contributing.md

changes to contributing.md
Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

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

I just noticed a few issues with the merged contributing guide.


- When documenting panics, errors, or other conceptual sections, introduce a Markdown section with a single `#`, e.g.:

- ```
Copy link
Contributor

Choose a reason for hiding this comment

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

There is an extraneous - here which messes up the formatting for the rest of the doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the indent had to be fixed here!

Comment on lines 361 to 371
#### Guidance for Slow/Non-Parallelizable Tests
PRs must include test coverage. However, if your PR includes large tests or tests which cannot run in parallel
(which is the default operation of the `cargo test` command), these tests should be decorated with `#[ignore]`.
If you add `#[ignore]` tests, you should add your branch to the filters for the `all_tests` job in our circle.yml
(or if you are working on net code or marf code, your branch should be named such that it matches the existing
filters there).

A test should be marked `#[ignore]` if:

1. It does not _always_ pass `cargo test` in a vanilla environment (i.e., it does not need to run with `--test-threads 1`).
2. Or, it runs for over a minute via a normal `cargo test` execution (the `cargo test` command will warn if this is not the case).
Copy link
Contributor

Choose a reason for hiding this comment

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

This sub-section should not be under the Comments section.

Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

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

lgtm!

@blockstack-devops
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@stacks-network stacks-network locked as resolved and limited conversation to collaborators Nov 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
L1 Working Group Issue or PR related to improving L1 locked
Projects
Status: Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants