feat: Add pulumi module #3055
feat: Add pulumi module #3055
Conversation
|
@iwahbe Thanks for the PR! It looks like our clippy check isn't happy about a Looking forward to getting this merged! |
Sorry about that. Fixed now. |
I implicitly assumed that if there was a valid (parsable) Pulumi.yaml, then it would have contents. We now allow it to be empty.
There was a problem hiding this comment.
Not gonna lie: I had no idea what pulumi did going into this, and after reading this PR and skimming the website, I'm still not 100% sure.
However, this looks generally pretty good. It looks like you missed adding a description for this in the description function (not really your fault--there's way too much to keep track of when adding a new module) and there's a few small changes/additions I'd like to see in the module itself.
docs/config/README.md
Outdated
| | ------------------- | ------------------------------------ | ------------------------------------------------------------------------- | | ||
| | `format` | `"via [$symbol$stack]($style) "` | The format string for the module. | | ||
| | `version_format` | `"v${raw}"` | The version format. Available vars are `raw`, `major`, `minor`, & `patch` | | ||
| | `symbol` | `"🚀 "` | A format string shown before the Pulumi stack. | |
There was a problem hiding this comment.
I must admit, I'm somewhat surprised this one hasn't been used already.
There was a problem hiding this comment.
Is there any connection between Pulumi and a rocket? Looking on their site, the default symbol looks pretty close to a cube. I wonder if one of the nerd-font cube symbols might be a better fit.
There was a problem hiding this comment.
I don't have strong opinions. I changed it to (which is a cube, when rendered)
| @@ -123,6 +124,7 @@ pub fn handle<'a>(module: &str, context: &'a Context) -> Option<Module<'a>> { | |||
| "package" => package::module(context), | |||
| "perl" => perl::module(context), | |||
| "php" => php::module(context), | |||
| "pulumi" => pulumi::module(context), | |||
| "purescript" => purescript::module(context), | |||
| "python" => python::module(context), | |||
| "rlang" => rlang::module(context), | |||
There was a problem hiding this comment.
We will also need a description for the module here. It would be near line 211 of this file, but the web interface won't let me actually mark that line, so the comment is going here :(
| @@ -0,0 +1,295 @@ | |||
| #![warn(missing_docs)] | |||
| use sha1::{Digest, Sha1}; | |||
There was a problem hiding this comment.
Just want to say in advance: I'm not at all familiar with pulumi, so forgive me if some of these questions are naive.
src/modules/pulumi.rs
Outdated
| /// can return results like `3.12.0-alpha.1630554544+f89e9a29.dirty`, which we | ||
| /// don't want to see. Instead we display that as `3.12.0-alpha`. | ||
| fn parse_version(version: &str) -> &str { | ||
| &version[0..version |
There was a problem hiding this comment.
While elegant in a functional sense, this takes a while to unpack and understand. Is there a reason we can't just increment a counter until we see three '.' characters (or hit the end of string) and then slice up to that index?
There was a problem hiding this comment.
I find the functional style easier to read. I will change it to be explicitly imperative on my next commit.
There was a problem hiding this comment.
For functional style, I personally would have gone with a split-take-join method, but I understand not wanting to cause additional allocations. Perhaps the thing to do to make this easier to understand would have been to name the last index with its own variable?
|
The hazards of pulling in additional crates. Looks like we have a build failure in zvariant, which is a dependency of our notification feature for long-running jobs. I suspect that this is a regression on nightly, since the rules on variable size shouldn't have changed recently. I think the simplest thing to do will be to wait a few days for |
|
rust-lang/rust#89125 got merged 8hrs ago. Hopefully the 2021-09-21 nightly builds correctly and then we'll be off to the races. |
|
Doesn't seem like. Hopefully it will be fixed it in the near future. |
|
It seems like all checks are passing now. |
|
@iwahbe Thank you for taking the time to write a PR! |
Description
Adds a module for Pulumi. The module can display:
Motivation and Context
It is useful to see what stack your commands are operating on (Can delete test, don't delete prod...). When hacking on or debugging pulumi, it is useful to see what version you are using.
Screenshots (if appropriate):
How Has This Been Tested?
In addition to unit and integration tests, I have been dogfooding this feature for about a week.
Checklist: