Skip to content

fix!: Rename certificates sha_1_fingerprint to sha1_fingerprint#2629

Merged
kodiakhq[bot] merged 2 commits intocloudquery:mainfrom
erezrokah:chore/codegen_drift_digitalocean
Oct 11, 2022
Merged

fix!: Rename certificates sha_1_fingerprint to sha1_fingerprint#2629
kodiakhq[bot] merged 2 commits intocloudquery:mainfrom
erezrokah:chore/codegen_drift_digitalocean

Conversation

@erezrokah
Copy link
Copy Markdown
Member

@erezrokah erezrokah commented Oct 11, 2022

Summary

Same as #2627 for DigitalOcean.

Renamed resources.go to main.go for consistency with other plugins. Also a breaking change due to the column rename.

@erezrokah erezrokah requested a review from a team October 11, 2022 14:15
@erezrokah erezrokah added the automerge Automatically merge once required checks pass label Oct 11, 2022
@hermanschaaf
Copy link
Copy Markdown
Contributor

hermanschaaf commented Oct 11, 2022 •
edited
Loading

We can probably override the column name in this case if we want to make the change non-breaking. Technically it's called SHA-1 (with a dash) so I think either with or without an underscore is fine. (Though our convention is generally to not have an underscore)

@erezrokah
Copy link
Copy Markdown
Member Author

erezrokah commented Oct 11, 2022

We can probably override the column name in this case if we want to make the change non-breaking. Technically it's called SHA-1 (with a dash) so I think either with or without an underscore is fine. (Though our convention is generally to not have an underscore)

That's a good point. sha1_fingerprint is taken from the JSON tag on the struct that's the reason for the change. I'm not sure we have a case we override the JSON tag.

@hermanschaaf
Copy link
Copy Markdown
Contributor

hermanschaaf commented Oct 11, 2022

@erezrokah We could override it in the recipe, right? Skip the field and add it manually with the name sha_1_fingerprint to make it non-breaking.

@hermanschaaf
Copy link
Copy Markdown
Contributor

hermanschaaf commented Oct 11, 2022

Not that it's a big deal. I'm good either way, if you'd prefer to make keep things consistent and make this a breaking change.

@erezrokah
Copy link
Copy Markdown
Member Author

erezrokah commented Oct 11, 2022 •
edited
Loading

Not that it's a big deal. I'm good either way, if you'd prefer to make keep things consistent and make this a breaking change.

Yeah, I think we always use the JSON tag if exists. If it didn't have one we could keep the the sha_1_. Also it's a very easy breaking change to handle

@kodiakhq kodiakhq bot merged commit 6f5aba9 into cloudquery:main Oct 11, 2022
@erezrokah erezrokah deleted the chore/codegen_drift_digitalocean branch October 11, 2022 14:29
@cq-bot cq-bot mentioned this pull request Oct 11, 2022
Merged
kodiakhq bot pushed a commit that referenced this pull request Oct 12, 2022
@cq-bot
chore(main): Release plugins-source-digitalocean v2.0.0 (#2559)
c867e67 
🤖 I have created a release *beep* *boop*
---


## [2.0.0](plugins-source-digitalocean-v1.0.1...plugins-source-digitalocean-v2.0.0) (2022-10-12)


### ⚠ BREAKING CHANGES

* Rename certificates `sha_1_fingerprint` to `sha1_fingerprint` (#2629)

### Bug Fixes

* **deps:** Update plugin-sdk for digitalocean to v0.12.10 ([#2548](#2548)) ([e26297e](e26297e))
* Rename certificates `sha_1_fingerprint` to `sha1_fingerprint` ([#2629](#2629)) ([6f5aba9](6f5aba9))
* Upgrade source SDK versions to v0.13.5 ([#2610](#2610)) ([611868e](611868e))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yevgenypats yevgenypats Awaiting requested review from yevgenypats

1 more reviewer

@hermanschaaf hermanschaaf hermanschaaf approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

automerge Automatically merge once required checks pass

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@erezrokah @hermanschaaf @cq-bot