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

Linting #263

Open
benbrittain opened this issue May 17, 2023 · 4 comments
Open

Linting #263

benbrittain opened this issue May 17, 2023 · 4 comments

Comments

@benbrittain
Copy link
Contributor

There is some discussion on the buck2 discord about best practices with linting and how to scale linting at smaller repo scales vs massive monorepos. The expectation of a massive monorepo tends to be that you have linters run on a CL upload. That works well with infra, but doesn't scale down to early stage open source projects.

Similar to RunInfo/DefaultInfo/InstallInfo, would there be any willingness to support something like a LintInfo that toolchains could use? This would allow folks to leverage the files changed graph that buck2 already has, and allow folks to drop into arbitrary buck2 projects and know how linting works.

@thoughtpolice
Copy link
Contributor

thoughtpolice commented May 20, 2023

I'm not sure increasing the surface area of the core API is needed for this. See an earlier report I wrote about UX improvements: #86 (please read my verbose-as-usual follow up comments aside from the original one)

If it's mostly about UX, which I think it is — then if this framework was in place, you could easily write a buck fmt or buck lint or buck fix command, backed by proper BXL wrappers, that can do necessary build introspection, run the tools, and commit the results, etc. You could already do this today, you just wouldn't have the awesome shorthand syntax. I think the question is something like — how does LintInfo differ in semantics from RunInfo? It's a bit odd... That said, this could all be prototyped with BXL, I think.

At small scales, I would say that tracking the files changed in the graph isn't really necessary from a performance POV. "Small" here means "It can fit on one computer", in my mind. Just using git status or git show or whatever and requiring any modified file have the formatter run on it is easy, can be added to your CI pipeline trivially, and ultimately doesn't require much more from buck than it can already do today.

@benbrittain
Copy link
Contributor Author

benbrittain commented May 20, 2023

I really like #86, I'll admit I've looked at BXL less than I should have. I think it could come really really close to a dedicated linting feature without expanding the core API as you said.

There is some benefit in having it provided by default at the start of setting up a project however. I think that could be solved the some sort of bxl script in prelude and thelint subcommand being set up by default on project init. It'd also let projects remove or totally rework what lint means which keeps flexibility.

@thoughtpolice
Copy link
Contributor

Yeah, thinking back on it this morning, I'm not even entirely convinced by my own argument! It would be a bit unfortunate to have everyone recreate these 2-3 commands over and over through BXL for every user...

Maybe there should be a lint command. But it just shouldn't do much and basically works like RunInfo for a target, it's just expected you're, well, running a linter! So having some more input/thoughts here would be good

@benbrittain
Copy link
Contributor Author

The biggest benefit of a dedicated lint command would be that it could also take a --fix param for language toolchains that support it. I don't know if the buck2 maintainers are comfortable with the build system modifying files in-tree though, I'm not sure I would be. It would be nice ergonomics though.

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

No branches or pull requests

2 participants