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

Use environment variable to set CODEOWNERS write path #114

Merged
merged 6 commits into from
Feb 3, 2025

Conversation

jibarra
Copy link
Contributor

@jibarra jibarra commented Jan 29, 2025

The current version of the gem always defaults CODEOWNERS to generate in .github/CODEOWNERS. This works well for projects that follow the same structure but doesn't work well for other structures (or other platforms). For example:

This change updates the path for the file location to pull from an environment variable (CODEOWNERS_PATH) to use for the path. It will still default to the old behavior of using .github/ to maintain backwards compatibility.

Copy link
Contributor

@ashleywillard ashleywillard left a comment

Choose a reason for hiding this comment

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

This seems like a good change. Another way to implement this would be to specify the codeowners path in the code_ownership.yml file.

@jibarra
Copy link
Contributor Author

jibarra commented Jan 31, 2025

@ashleywillard - I like that recommendation a little better. I'll see about doing that instead.

@jibarra
Copy link
Contributor Author

jibarra commented Jan 31, 2025

@ashleywillard - How's this look now?

@jibarra jibarra requested a review from ashleywillard January 31, 2025 23:09
Copy link
Contributor

@ashleywillard ashleywillard 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! cc @perryqh

@ashleywillard ashleywillard merged commit 2b233e5 into rubyatscale:main Feb 3, 2025
6 checks passed
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

Successfully merging this pull request may close these issues.

2 participants