Skip to content

feat: add name.com provider #474

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jun 15, 2023
Merged

feat: add name.com provider #474

merged 4 commits into from
Jun 15, 2023

Conversation

jdiekhoff
Copy link
Contributor

Implementing feature request #218 to add support for name.com as a provider.

qdm12
qdm12 reviewed Jun 5, 2023
View reviewed changes
@jdiekhoff
Copy link
Contributor Author

@qdm12 review suggestions applied and fixed a lint error.

qdm12
qdm12 reviewed Jun 7, 2023
View reviewed changes
Copy link
Owner

@qdm12 qdm12 left a comment

Choose a reason for hiding this comment

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

Re-reviewed, there are a few changes/additions needed, sorry I didn't dig too much on the first review. Thanks! 👍

@qdm12 qdm12 changed the title Adding support for name.com feat: add name.com provider Jun 7, 2023
@jdiekhoff
Copy link
Contributor Author

Re-reviewed, there are a few changes/additions needed, sorry I didn't dig too much on the first review. Thanks! 👍

Will look into these over the next couple of days, appreciate the thoroughness.

jdiekhoff and others added 3 commits June 14, 2023 07:13
@jdiekhoff @qdm12
Adding support for name.com
ead506d
@jdiekhoff @qdm12
Updating error handling
1e1dca2
@qdm12
Apply PR changes
6d1dd43 
- Make TTL optional
- Change TTL type to be `*uint32` to match name.com API documentation
- Validate TTL is 300 at least, if set
- Set content type application/json header to requests
- Set user agent header to requests
- Check response received for ip mismatch
- Rename password -> token
- docs/name.com.md: remove domain setup section
- docs/name.com.md: inline API credentials section in compulsory parameters
- Inline isValid function in constructor
- Handle common status code errors and error messages
- Other minor PR feedback
@qdm12
Copy link
Owner

qdm12 commented Jun 14, 2023

I rebased the branch and solved the PR feedback I left 😉

You can get changes on your fork with git fetch && git reset --hard origin/namedotcom, and then can you try the image see if it still works fine?
It could also be interesting to try it with wrong credentials to see how it handles errors. Thanks!!

@jdiekhoff
Copy link
Contributor Author

@qdm12 had to make a slight change in the create logic. Previously if a record was created after not being found, the logic was setup so that the update method would be called. This resulted in an error as no recordID is included at this point so the update will fail.

Create_Followed_By_Bad_Update

Also checked on using wrong credentials and a clean message is displayed:

Bad_Auth

@qdm12
Copy link
Owner

qdm12 commented Jun 15, 2023

Awesome, thanks! Merging ahead! 😉

@qdm12 qdm12 merged commit 06b6288 into qdm12:master Jun 15, 2023
@jdiekhoff jdiekhoff deleted the namedotcom branch June 15, 2023 11:45
@qdm12 qdm12 mentioned this pull request Jun 15, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qdm12 qdm12 qdm12 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jdiekhoff @qdm12