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.

Sign up for GitHub

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

Jump to bottom

[lint] Make aptos move lint return non-zero exit code on warnings and errors #15761

Merged
merged 1 commit into from
Jan 16, 2025
Merged

[lint] Make aptos move lint return non-zero exit code on warnings and errors #15761

merged 1 commit into from
Jan 16, 2025

Conversation

vineethk
Copy link
Contributor

@vineethk vineethk commented Jan 16, 2025 •
edited
Loading

Description

With this PR, aptos move lint returns exit code 0 on success and 1 if there are any warnings or errors.

This was a feature request by @banool for ease of use in CI/scripts.

How Has This Been Tested?

Manually tested on a projects that (a) produce warnings and (b) do not produce warnings, and verified we have expected behavior.

Type of Change

  • New feature

Which Components or Systems Does This Change Impact?

  • Aptos CLI/SDK
  • Move Linter

@trunk-io Trunk.io
Copy link

trunk-io bot commented Jan 16, 2025 •
edited
Loading

⏱️ 56m total CI duration on this PR
Slowest 15 Jobs Cumulative Duration Recent Runs
execution-performance / single-node-performance 25m 🟩 
execution-performance / test-target-determinator 7m 🟩 
rust-doc-tests 7m 🟩 
test-target-determinator 5m 🟩 
check-dynamic-deps 4m 🟩 🟩 
rust-cargo-deny 3m 🟩 🟩 
fetch-last-released-docker-image-tag 2m 🟩 
semgrep/ci 54s 🟩 🟩 
general-lints 52s 🟩 🟩 
file_change_determinator 24s 🟩 🟩 
file_change_determinator 11s 🟩 
permission-check 6s 🟩 🟩 
permission-check 3s 🟩 🟩 
permission-check 2s 🟩 
determine-docker-build-metadata 1s 🟩 

🚨 2 jobs on the last run were significantly faster/slower than expected

Job Duration vs 7d avg Delta
execution-performance / single-node-performance 25m 16m +63%
execution-performance / test-target-determinator 7m 5m +56%

settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io

@vineethk Graphite App
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@vineethk vineethk changed the title Linter returns non-zero exit code on warnings [lint] Make aptos move lint returns non-zero exit code on warnings and errors Jan 16, 2025
@vineethk vineethk changed the title [lint] Make aptos move lint returns non-zero exit code on warnings and errors [lint] Make aptos move lint return non-zero exit code on warnings and errors Jan 16, 2025
@vineethk vineethk marked this pull request as ready for review January 16, 2025 22:15
@vineethk vineethk requested review from banool, rahxephon89 and fEst1ck and removed request for davidiw and movekevin January 16, 2025 22:15
@vineethk vineethk enabled auto-merge (squash) January 16, 2025 22:31
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions
Copy link
Contributor

✅ Forge suite realistic_env_max_load success on af7df439dc290fdb121161d16f42e13d4582ae81

two traffics test: inner traffic : committed: 13619.69 txn/s, submitted: 13628.37 txn/s, expired: 8.68 txn/s, latency: 2877.05 ms, (p50: 2700 ms, p70: 3000, p90: 3300 ms, p99: 4800 ms), latency samples: 5178540
two traffics test : committed: 99.99 txn/s, latency: 1368.26 ms, (p50: 1300 ms, p70: 1400, p90: 1500 ms, p99: 2400 ms), latency samples: 1660
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 1.799, avg: 1.648", "ConsensusProposalToOrdered: max: 0.317, avg: 0.306", "ConsensusOrderedToCommit: max: 0.351, avg: 0.330", "ConsensusProposalToCommit: max: 0.666, avg: 0.636"]
Max non-epoch-change gap was: 1 rounds at version 2076780 (avg 0.00) [limit 4], 1.91s no progress at version 2076780 (avg 0.21s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.62s no progress at version 2061950 (avg 0.59s) [limit 16].
Test Ok

@github-actions GitHub Actions
Copy link
Contributor

✅ Forge suite compat success on 6593fb81261f25490ffddc2252a861c994234c2a ==> af7df439dc290fdb121161d16f42e13d4582ae81

Compatibility test results for 6593fb81261f25490ffddc2252a861c994234c2a ==> af7df439dc290fdb121161d16f42e13d4582ae81 (PR)
1. Check liveness of validators at old version: 6593fb81261f25490ffddc2252a861c994234c2a
compatibility::simple-validator-upgrade::liveness-check : committed: 17304.68 txn/s, latency: 2025.61 ms, (p50: 2100 ms, p70: 2100, p90: 2300 ms, p99: 2700 ms), latency samples: 556960
2. Upgrading first Validator to new version: af7df439dc290fdb121161d16f42e13d4582ae81
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 4756.61 txn/s, latency: 6500.93 ms, (p50: 7200 ms, p70: 8000, p90: 8300 ms, p99: 8300 ms), latency samples: 96740
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 4871.10 txn/s, latency: 7088.01 ms, (p50: 7800 ms, p70: 7900, p90: 8200 ms, p99: 8300 ms), latency samples: 173500
3. Upgrading rest of first batch to new version: af7df439dc290fdb121161d16f42e13d4582ae81
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 4318.02 txn/s, latency: 7215.50 ms, (p50: 8100 ms, p70: 8700, p90: 9100 ms, p99: 9300 ms), latency samples: 92580
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 4355.34 txn/s, latency: 7805.63 ms, (p50: 8600 ms, p70: 9000, p90: 9300 ms, p99: 9400 ms), latency samples: 151880
4. upgrading second batch to new version: af7df439dc290fdb121161d16f42e13d4582ae81
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 7903.59 txn/s, latency: 3815.47 ms, (p50: 4200 ms, p70: 4500, p90: 5200 ms, p99: 5400 ms), latency samples: 146020
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 7904.07 txn/s, latency: 4306.63 ms, (p50: 4300 ms, p70: 5200, p90: 5400 ms, p99: 5800 ms), latency samples: 262760
5. check swarm health
Compatibility test for 6593fb81261f25490ffddc2252a861c994234c2a ==> af7df439dc290fdb121161d16f42e13d4582ae81 passed
Test Ok

@vineethk vineethk merged commit 4b69e48 into main Jan 16, 2025
89 of 90 checks passed
@vineethk vineethk deleted the vk/linter-non-zero-exit-if-warnings branch January 16, 2025 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rahxephon89 rahxephon89 rahxephon89 approved these changes

@banool banool banool approved these changes

@wrwg wrwg Awaiting requested review from wrwg wrwg is a code owner

@fEst1ck fEst1ck Awaiting requested review from fEst1ck

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vineethk @banool @rahxephon89