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

RELEASE: Move some BYOS steps to be standard #3107

Merged
merged 8 commits into from
Sep 17, 2024
Merged

RELEASE: Move some BYOS steps to be standard #3107

merged 8 commits into from
Sep 17, 2024

Conversation

tlimoncelli
Copy link
Contributor

No description provided.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Changes detected by Check Git Status Action

documentation/writing-providers.md Outdated Show resolved Hide resolved

1. Add the provider to the `PROVIDERS` list.

* Add the name of the provider to the PROVIDERS list.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question: Is the idea to create a sublist? Then this content should get an extra indentation.

Suggested change
* Add the name of the provider to the PROVIDERS list.
* Add the name of the provider to the PROVIDERS list.

Screenshot 2024-09-11 at 13 05 47{width=750px}

https://docs.dnscontrol.org/~/revisions/uwTDnrDdAmna8AzbdoCc/developer-info/writing-providers#step-14-update-pr_test.yml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The two lines are a duplicate. i've removed one.

documentation/writing-providers.md Outdated Show resolved Hide resolved
documentation/writing-providers.md Outdated Show resolved Hide resolved
documentation/writing-providers.md Outdated Show resolved Hide resolved
documentation/writing-providers.md Show resolved Hide resolved
@@ -317,6 +368,7 @@ These are the things we'll be checking when you submit the PR. Please try to co
5. Re-run the [integration test](#step-7-integration-test) one last time.
* Post the results as a comment to your PR.
6. Re-read the [maintainer's responsibilities](providers.md#providers-with-contributor-support) bullet list. By submitting a provider you agree to maintain it, respond to bugs, periodically re-run the integration test to verify nothing has broken, and if we don't hear from you for 2 months we may disable the provider.
7. [Create an issue (feature request)](https://github.com/StackExchange/dnscontrol/issues/new?title=Add%20label%20for%20PROVIDERNAME) with the text "Please create the GitHub label 'provider-PROVIDERNAME'".
Copy link
Collaborator

Choose a reason for hiding this comment

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

Idea: Is it an idea to put this more as a checklist for us? Also in a review I feel the need to be able to tick off a checklist.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It might make more sense to include the (create the GitHub label 'provider-PROVIDERNAME') request in the original GitHub pull request.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My philosophy is that if someone's work is going to be judged using a rubric, they should know what the rubric is ahead of time. This helps them get everything right on the first try.

I've moved the label request to the PR. Good idea!

tlimoncelli and others added 3 commits September 11, 2024 09:03
Co-authored-by: Jeffrey Cafferata <jeffrey@jcid.nl>
Co-authored-by: Jeffrey Cafferata <jeffrey@jcid.nl>
Co-authored-by: Jeffrey Cafferata <jeffrey@jcid.nl>
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Changes detected by Check Git Status Action

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Changes detected by Check Git Status Action

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Changes detected by Check Git Status Action

@cafferata
Copy link
Collaborator

@tlimoncelli, can you let me know when I can do a review? 🤓

Copy link
Contributor Author

@tlimoncelli tlimoncelli left a comment

Choose a reason for hiding this comment

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

I had to undo the PROVIDERS list change. It is a long string, not a list.


1. Add the provider to the `PROVIDERS` list.

* Add the name of the provider to the PROVIDERS list.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The two lines are a duplicate. i've removed one.

documentation/writing-providers.md Outdated Show resolved Hide resolved
documentation/writing-providers.md Show resolved Hide resolved
@@ -317,6 +368,7 @@ These are the things we'll be checking when you submit the PR. Please try to co
5. Re-run the [integration test](#step-7-integration-test) one last time.
* Post the results as a comment to your PR.
6. Re-read the [maintainer's responsibilities](providers.md#providers-with-contributor-support) bullet list. By submitting a provider you agree to maintain it, respond to bugs, periodically re-run the integration test to verify nothing has broken, and if we don't hear from you for 2 months we may disable the provider.
7. [Create an issue (feature request)](https://github.com/StackExchange/dnscontrol/issues/new?title=Add%20label%20for%20PROVIDERNAME) with the text "Please create the GitHub label 'provider-PROVIDERNAME'".
Copy link
Contributor Author

Choose a reason for hiding this comment

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

My philosophy is that if someone's work is going to be judged using a rubric, they should know what the rubric is ahead of time. This helps them get everything right on the first try.

I've moved the label request to the PR. Good idea!

@tlimoncelli
Copy link
Contributor Author

@cafferata PTAL

@tlimoncelli tlimoncelli merged commit 269542c into main Sep 17, 2024
18 of 19 checks passed
@tlimoncelli tlimoncelli deleted the tlim_newprov branch September 17, 2024 00:54
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.

2 participants