Skip to content
New issue

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

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

Already on GitHub? # to your account

chore(ci): improving CI #301

Merged
merged 19 commits into from
Jun 23, 2023
Merged

chore(ci): improving CI #301

merged 19 commits into from
Jun 23, 2023

Conversation

jeertmans
Copy link
Collaborator

Hello @maciejhirsz,

This PR proposes new changes regarding continuous integration tests, that I think should facilitate maintaining this project.

Here are the main advantages:

  • improved speed for workflows with cache intensive usage; and a specific workflow is used to clear cache on closed PR (clearcache.yml);
  • code coverage (tests), with badge (e.g.: see my README) and PR reports (e.g.: test(ci): add dumb token jeertmans/logos#2 (comment)). Setup is very easy: https://app.codecov.io/;
  • benchmarking PR before/after changes (e.g.: test(ci): add dumb token jeertmans/logos#2 (comment));
  • test Rust minimal support version (MSRV). In my opinion, this is always good to document the MSRV for your project, and a workflows automates it;
  • tests are split in multiple workflows and jobs, so that contributors easily understand what fails;
  • and many more tests are provided.

A few remarks:

  • I update Rust's version and Rust's edition;
  • benchmarks are now performed using Criterion (it automatically compares benchmarks);
  • the benchmark action should fail here, this is because the master base branch does not contain the expected benchmarks; it will work after merging;
  • you need to setup https://app.codecov.io/ for the code coverage upload to work;
  • and Clippy's check should fail, because it already fails on the base branch. But I think fixing its warning should be for another PR.

Do no hesitate to give your opinion on this :-)

* chore(ci): reorganize CI

* fix(ci): add rustfmt component

* chore(ci): add benchmarks

* try(ci): fixing ?

* fix(ci): indent

* fix(ci): trying :(

* fix(ci): caching and cat

* fix(ci): echo pwd

* fix(ci): try ...

* chore(ci): run on head and on base

* fix(ci): git checkout manually

* chore(ci): update benches

* chore(ci): fix

* fix(ci): checkout base actually

* fix(ci): use checkout action
@maciejhirsz
Copy link
Owner

Thanks @jeertmans! I'll have a proper look at this hopefully later today.

I'm trying to manage my time a bit better, and I'm thinking about carving out one day a week for a general maintenance sweet across my crates, tentatively looking at Fridays. I reckon the pace being slow here will be much more manageable if it's at least predictable.

@jeertmans
Copy link
Collaborator Author

jeertmans commented Apr 21, 2023

No problem @maciejhirsz
I hope this PR will also facilitate reviewing future PRS :D

@jeertmans jeertmans mentioned this pull request Apr 21, 2023
13 tasks
jeertmans and others added 2 commits April 26, 2023 15:15
* chore(codegen): error to prevent undesired behavior

* chore(deps): add lazy static

* chore(codegen): apply suggested changes
* it's -> its

* Update internal.rs

* Update lexer.rs

* Update lib.rs
* chore(ci): reorganize CI

* fix(ci): add rustfmt component

* chore(ci): add benchmarks

* try(ci): fixing ?

* fix(ci): indent

* fix(ci): trying :(

* fix(ci): caching and cat

* fix(ci): echo pwd

* fix(ci): try ...

* chore(ci): run on head and on base

* fix(ci): git checkout manually

* chore(ci): update benches

* chore(ci): fix

* fix(ci): checkout base actually

* fix(ci): use checkout action
@jeertmans
Copy link
Collaborator Author

Results for before/after changes on applying clippy's suggestions

group                                         before                                 changes
-----                                         ------                                 -------
count_ok/identifiers                          1.00   488.3±20.25ns  1521.4 MB/sec    1.01   495.0±15.08ns  1500.9 MB/sec
count_ok/keywords_operators_and_punctators    1.01  1366.1±11.08ns  1487.6 MB/sec    1.00  1350.6±42.38ns  1504.7 MB/sec
count_ok/strings                              1.00   536.3±10.77ns  1548.8 MB/sec    1.00   537.9±18.65ns  1544.4 MB/sec
iterate/identifiers                           1.00   485.4±13.38ns  1530.5 MB/sec    1.05   509.0±19.42ns  1459.7 MB/sec
iterate/keywords_operators_and_punctators     1.00  1342.7±41.67ns  1513.6 MB/sec    1.01  1356.0±32.37ns  1498.8 MB/sec
iterate/strings                               1.00   545.3±15.31ns  1523.3 MB/sec    1.00    542.9±7.48ns  1530.1 MB/sec

Conclusion

It did not affect performances (as expected).

@jeertmans
Copy link
Collaborator Author

@jeertmans, I'm merging this in the hope it will accelerate developing the book and the "new" Logos, thanks to a better test-suite.

If you are okay with test coverage, you still need to setup (free) the codecov app:

Once done, we can add a badge on the README.

@jeertmans jeertmans merged commit b1406db into maciejhirsz:master Jun 23, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants