Skip to content

Add experimental support for Rustfmt#18842

Merged
tdyas merged 79 commits intopantsbuild:mainfrom
lassemand:main
Jun 7, 2023
Merged

Add experimental support for Rustfmt#18842
tdyas merged 79 commits intopantsbuild:mainfrom
lassemand:main

Conversation

@lassemand
Copy link
Contributor

@lassemand lassemand commented Apr 27, 2023

Initial PR for providing rust support. This PR is limited to providing rustfmt functionality strictly.

Idea: Use rustfmt to provide fmt functionality

I consider getting Sources from cargo.toml out of scope for this PR.

@lassemand lassemand marked this pull request as ready for review April 28, 2023 09:47
Copy link
Contributor

@huonw huonw left a comment

Choose a reason for hiding this comment

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

Thanks for continuing on Rust support. Are you aware of previous efforts like #15093, and the discussion at #15775 and #17502?

@lassemand
Copy link
Contributor Author

Thanks for continuing on Rust support. Are you aware of previous efforts like #15093, and the discussion at #15775 and #17502?

Yes, I have talked to tdyas while making this PR, I have also been taking a look at the https://docs.google.com/document/d/1Q5h0PTusb2WyD3EScgeqUxBPF1IuEXCaa3qO31HEHjQ as well as taking a lot of inspiration from the go environment.

I would like to limit the scope of this PR, and hence I prefer to put the current progress as is.

@stuhood stuhood requested a review from tdyas April 28, 2023 19:46
@stuhood
Copy link
Member

stuhood commented Apr 28, 2023

@lassemand: Thanks a lot!

@tdyas: Let me know if you'd like another reviewer.

Copy link
Contributor

@tdyas tdyas left a comment

Choose a reason for hiding this comment

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

General comment is that we should figure out what the target types are going to be and which goals map to them. Otherwise introducing rust_crate here as the focus for fmt might need to change in the future.

I was envisioning target types inspired by the Go backend:

  • rust_package maps to go_mod
  • rust_crate maps to go_binary

Since cargo fmt applies to the Rust package, in that view, pants fmt should map to rust_package which encompasses the whole Rust package.

Are you envisioning a different set of target types?

@lassemand
Copy link
Contributor Author

@thejcannon what is the next step on this PR?

@thejcannon
Copy link
Member

I have a reminder to take another look tomorrow morning.

Regardless of comments we're probably close to merging because you've been a trooper along this route and we can just fix anything if it isn't egregious.

Copy link
Contributor

@tdyas tdyas left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for the effort here!

@tdyas
Copy link
Contributor

tdyas commented May 30, 2023

I'll merge this tomorrow.

Copy link
Member

@thejcannon thejcannon left a comment

Choose a reason for hiding this comment

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

Just some dead code, otherwise, sweet!!

@lassemand lassemand requested a review from thejcannon June 2, 2023 09:48
@tdyas
Copy link
Contributor

tdyas commented Jun 2, 2023

@lassemand: Is this ready to be merged?

@lassemand
Copy link
Contributor Author

@lassemand: Is this ready to be merged?

Imo it is ready yes

@lassemand
Copy link
Contributor Author

@tdyas I tagged @thejcannon since he had I comment which I prefer not to address

@tdyas tdyas merged commit 31dc868 into pantsbuild:main Jun 7, 2023
alonsodomin pushed a commit to alonsodomin/pants that referenced this pull request Jun 8, 2023
Initial PR for providing rust support. This PR is limited to providingrustfmt functionality strictly.

Idea: Use rustfmt to provide fmt functionality

I consider getting Sources from cargo.toml out of scope for this PR.

---------

Co-authored-by: Lasse Alm <lama@trifork.com>
Co-authored-by: Jacob Floyd <cognifloyd@gmail.com>
huonw added a commit that referenced this pull request Jun 14, 2023
This updates `changelog.py` to write the (non-internal) changes to the
relevant release notes file directly, rather than requiring a human to
copy-paste it in. For now, the file is just mutated, without committing.
The internal changes are still printed for the human to deal with.

For instance, `pants run src/python/pants_release/changelog.py --
--prior 2.18.0.dev1 --new 2.18.0.dev2` adds a new section to
`src/python/pants/notes/2.18.x.md`:

```diff
diff --git a/src/python/pants/notes/2.18.x.md b/src/python/pants/notes/2.18.x.md
index e648a45..d6668a24b1 100644
--- a/src/python/pants/notes/2.18.x.md
+++ b/src/python/pants/notes/2.18.x.md
@@ -1,5 +1,35 @@
 # 2.18.x Release Series
 
+## 2.18.0.dev2 (Jun 14, 2023)
+
+### New Features
+
+* Include complete platforms for FaaS environments for more reliable building ([#19253](#19253))
+
+* Add experimental support for Rustfmt ([#18842](#18842))
+
+* Helm deployment chart field ([#19234](#19234))
+
+### Plugin API Changes
+
+* Replace `include_special_cased_deps` flag with `should_traverse_deps_predicate` ([#19272](#19272))
+
+### Bug Fixes
+
+* Raise an error if isort can't read a config file ([#19294](#19294))
+
+* Improve handling of additional files in Helm unit tests ([#19263](#19263))
+
+* Add taplo to the release ([#19258](#19258))
+
+* Handle `from foo import *` wildcard imports in Rust dep inference parser ([#19249](#19249))
+
+* Support usage of `scala_artifact` addresses in `scalac_plugin` targets ([#19205](#19205))
+
+### Performance
+
+* `scandir` returns `Stat`s relative to its directory. ([#19246](#19246))
+
 ## 2.18.0.dev1 (Jun 02, 2023)
 
 ### New Features
```

This also moves it into the new `pants_release` root, adds some basic
tests, and has it fetch the relevant branch directly, rather than
prompting the user to do so. It also pulls out a `git.py` support module
with helpers for exec-ing `git` as an external process.

The commits are individually reviewable.

This is a step towards automating more of the "start release" process,
per #19279. After this
core refactoring of the functionality, I think the next step is to
combine it with the CONTRIBUTORS update and version bumping, and then
after that string it all together on CI so that starting a release is
fully automated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants