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

Move nightly toolchain scripts to xtasks #1790

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

heaths
Copy link
Member

@heaths heaths commented Sep 3, 2024

This is an emerging pattern many other projects are using and allows us to develop them with current rust-analyzer and friends, which rust scripts - requiring the nightly toolchain - do not.

This is an emerging pattern many other projects are using and allows us to develop them with current rust-analyzer and friends, which rust scripts - requiring the nightly toolchain - do not.
@heaths
Copy link
Member Author

heaths commented Sep 3, 2024

Despite .editorconfig files using a TOML-like syntax, seems I won't be able to use the toml crate because it strictly enforces some rules like "valid" section names and quoted strings. I don't want to pull in a lot of dependencies because cargo xtasks should be quick to build. Perhaps I should go the route of making each a separate EXE to minimize dependencies for some tasks at least.

@analogrelay
Copy link
Member

Despite .editorconfig files using a TOML-like syntax, seems I won't be able to use the toml crate because it strictly enforces some rules like "valid" section names and quoted strings. I don't want to pull in a lot of dependencies because cargo xtasks should be quick to build.

Looks like configparser would work. Agreed in general about keeping dependencies minimal, but dependency builds should be cached for the user (even with xtasks) so I'm not that worried about adding one more if it avoids custom parsing stuff.

This is an emerging pattern many other projects are using and allows us to develop them with current rust-analyzer and friends, which rust scripts - requiring the nightly toolchain - do not.

I like this pattern, particularly as it avoids nightly rust as much as possible. In general, I prefer writing scripts using the toolchain if possible anyway, since they can be cross-platform that way.

@heaths
Copy link
Member Author

heaths commented Sep 3, 2024

I'm not sure about this approach either. As our repo grows, putting more and more into the workspace - especially with dependencies not generally needed by other crates (which we want to reduce anyway) - is going to make things slower and slower. Rust scripts will stabilize eventually and editor support should come along with that, so this may not be worth it.

@heaths
Copy link
Member Author

heaths commented Sep 5, 2024

Looks like configparser would work. Agreed in general about keeping dependencies minimal, but dependency builds should be cached for the user (even with xtasks) so I'm not that worried about adding one more if it avoids custom parsing stuff.

Looks like it's active as well. I did find a crate for helping assert files against an .editorconfig but it hasn't been touched for 3 years. Unfortunately, this is all too common. I may also just look into EditorConfig-recomended JS libraries we can run at build time and see if I can instead lint a header. I started work on one a while back but shelved it for a time. Based on dylint, which I think we'd want long-term anyway just like Azure SDKs for .NET add Roslyn analyzers as needed.

Could also use something like https://docs.rs/file-header/latest/file_header/fn.check_headers_recursively.html, which looks to be maintained. The path predicate could use a list of files passed to the command line including globs or, if none, check every *.rs file in the workspace at least. This way, CI can pass a list of changed file and it would run faster. Not sure how easy that is for them at the moment, but I know they at least drop a JSON file we could parse as well. @hallipr?

The advantage of a lint e.g., using dylint is that feedback is immediate in most editors.

@analogrelay
Copy link
Member

Another option for linting is super-linter. I've worked with some projects that use it. It's essentially a bundle of linters. It can be a bit of a heavy option but has the advantage of just setting a style baseline for everything.

@heaths heaths changed the base branch from feature/track2 to main October 18, 2024 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants