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

Strip Provider Prefix from File Names during Migration #349

Merged
merged 5 commits into from Mar 28, 2024

Conversation

SBGoods
Copy link
Contributor

@SBGoods SBGoods commented Mar 27, 2024

Closes: #347
Supersedes: #348

Fixes a bug that is encountered when the migrate command and generate command are run on provider documentation that have provider short name prefixes in their filenames (ex. time_rotating.md). The migrate command would move the files to the template folder as-is but the generate command does not recognize these files as valid documentation files because of the prefix, and therefore generates default template for the "missing" resource/data-sources, resulting in duplicate templates. This PR resolves this by introducing a --provider-name flag to the migrate subcommand, and uses the provided value to derive the provider short name and strips the prefix from an file name containing it.

The --provider-name flag is optional for users who do not have the provider prefix in their file names, so they can continue to run the migrate command without any flags, if the defaults are appropriate.

@SBGoods SBGoods requested a review from a team as a code owner March 27, 2024 20:04
Copy link
Member

@austinvalle austinvalle left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 :shipit:

@bflad
Copy link
Member

bflad commented Mar 28, 2024

Non-blocking: Should migrate --provider-name default to the short name of the provider directory similar to generate --provider-name here?

if g.providerName == "" {
g.providerName = filepath.Base(g.providerDir)
}

I think if migrate can do the expected behavior of stripping the prefixes automatically, but potentially leaving an opt-out for developers to skip that default behavior, it could prevent confusion.

@SBGoods
Copy link
Contributor Author

SBGoods commented Mar 28, 2024

@bflad Good shout! I completely forgot about the provider directory name. I've updated the code to use it as a default, and tweaked the documentation to make it clear that the prefixes are removed by default.

Copy link
Member

@bflad bflad 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 🚀

.changes/unreleased/ENHANCEMENTS-20240327-160831.yaml Outdated Show resolved Hide resolved
Co-authored-by: Brian Flad <bflad417@gmail.com>
@SBGoods SBGoods merged commit e3fab9a into main Mar 28, 2024
6 checks passed
@SBGoods SBGoods deleted the strip-provider-prefix branch March 28, 2024 19:45
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate docs are generated when template names have <provider-name>_ prefix
3 participants