-
Notifications
You must be signed in to change notification settings - Fork 43
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
Add option to filter edit rules by location in docs conversion pipeline and apply for installation docs #2435
Conversation
Add complete file tests, with and without provider-supplied edits
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2435 +/- ##
==========================================
- Coverage 57.43% 57.12% -0.31%
==========================================
Files 369 370 +1
Lines 50209 51030 +821
==========================================
+ Hits 28836 29152 +316
- Misses 19795 20289 +494
- Partials 1578 1589 +11 ☔ View full report in Codecov by Sentry. |
104a538
to
507aa1b
Compare
507aa1b
to
c55ae3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When edit rules are applied is important. Can we add a note to info.DocsEdit
that tells the user "your edit will be applied before any other edits"?
I think the change is OK as is, but it also feels like this makes edit rules application even more complicated and special for installation docs.
We can merge as is if we really need to, but I'd love if we took the opportunity to re-unify behavior for edit rules. For that, see 👇.
Instead of making bridge supplied edit rules special, I propose that we:
- Add a field to
info.DocsEdit
to control where the edit rule is applied. Initially, I would suggest an enum with:EditPreTranslation
(the zero value, since this is what we do for normal pages) andEditPostTranslation
. - Edit
apply
to also take a phase (pre or post) and then filter edit rules applied. - Move the existing "special" edit rules for installation docs into
defaultEditRules
(in edit_rules.go), specifyingEditPostTranslation
as needed.
Hmm, I like this idea, and I think that edit rules should be applied mostly after code conversion. The main case for applying them pre-conversion is to fix any mistakes in the provided code, as was discovered here. Having both options would not only allow us to fix incorrect TF code and make it convertable (convertible? 🚗 ), but also give us far more freedom in translating "Terraform" strings for "Pulumi" ones. |
…ion constants and apply everywhere.
Refactor tests to run directly on editRules.apply. Make removeTfVersionMentions and skipSectionHeadersEdit match any path, as we'll specify the index file when calling g.editRules.apply
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some comment nits, but otherwise LGTM
This PR has been shipped in release v3.92.0. |
This code is required to successfully apply the edit rules needed for pulumi/pulumi-libvirt#426.
It covers the case for when the upstream file's code sections need to be adjusted for code conversion to work.
It adds a complete index file test which tests the entire
plainDocsParser
function.In support of https://github.com/pulumi/home/issues/3598.