Skip to content

fix: Move source & destination plugin code to separate packages#516

Merged
kodiakhq[bot] merged 2 commits intomainfrom
split_source_test
Dec 18, 2022
Merged

fix: Move source & destination plugin code to separate packages#516
kodiakhq[bot] merged 2 commits intomainfrom
split_source_test

Conversation

@yevgenypats
Copy link
Copy Markdown
Contributor

@yevgenypats yevgenypats commented Dec 17, 2022 •
edited
Loading

Im trying to add more functionality and tests to our destination plugin API but this because quite hard now when source and plugin reside in the same place and everything is suffixed with source or destination.

This should make it more "go" idiomatic by splitting source and destination into two different packages and help with follow-up PRs.

This is a breaking change but relatively minor one as it is mostly affecting one file in each plugin (the main.go or plugin.go)

@github-actions github-actions bot added breaking and removed fix labels Dec 17, 2022
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 17, 2022 •
edited
Loading

⏱️ Benchmark results

Comparing with a0b8a46

  • DefaultConcurrency-2 resources/s: 11,921 ⬇️ 0.34% decrease vs. a0b8a46
  • Glob-2 ns/op: 190.8 ⬇️ 12.05% decrease vs. a0b8a46
  • TablesWithChildrenDefaultConcurrency-2 resources/s: 28,132 ⬆️ 4.94% increase vs. a0b8a46
  • BufferedScanner-2 ns/op: 11.21 ⬇️ 16.50% decrease vs. a0b8a46
  • LogReader-2 ns/op: 36.69 ⬇️ 14.04% decrease vs. a0b8a46

@candiduslynx candiduslynx changed the title fix!: Move source,destination plugin code to seperate packages fix!: Move source & destination plugin code to separate packages Dec 17, 2022
@github-actions github-actions bot added breaking and removed fix labels Dec 17, 2022
@candiduslynx
Copy link
Copy Markdown
Contributor

candiduslynx commented Dec 17, 2022

As this is a breaking change, the mod path should be suffixed with /v2
@yevgenypats @erezrokah

@yevgenypats
Copy link
Copy Markdown
Contributor Author

yevgenypats commented Dec 17, 2022

No need to move to v2 on every breaking change unless it is significant in our case as most of the plugins are maintained by us anyway so this will just make us more work.

@erezrokah erezrokah changed the title fix!: Move source & destination plugin code to separate packages fix: Move source & destination plugin code to separate packages Dec 18, 2022
@github-actions github-actions bot added fix and removed fix labels Dec 18, 2022
@erezrokah erezrokah changed the title fix: Move source & destination plugin code to separate packages fix!: Move source & destination plugin code to separate packages Dec 18, 2022
@github-actions github-actions bot added breaking and removed fix labels Dec 18, 2022
erezrokah
erezrokah approved these changes Dec 18, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@erezrokah erezrokah left a comment •
edited
Loading

Choose a reason for hiding this comment

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

By making this a breaking change it will bump the Go package to v2 and we'll need to update the imports for all plugins? Do we want that?

I'm good either way (not making it breaking and only fixing complication errors)

@yevgenypats yevgenypats changed the title fix!: Move source & destination plugin code to separate packages fix: Move source & destination plugin code to separate packages Dec 18, 2022
@github-actions github-actions bot removed fix labels Dec 18, 2022
@github-actions github-actions bot added the fix label Dec 18, 2022
@yevgenypats
Copy link
Copy Markdown
Contributor Author

yevgenypats commented Dec 18, 2022

By making this a breaking change it will bump the Go package to v2 and we'll need to update the imports for all plugins? Do we want that?

I'm good either way (not making it breaking and only fixing complication errors)

We don't want to move to v2. I marked it as non breaking.

@kodiakhq kodiakhq bot merged commit 6733785 into main Dec 18, 2022
@kodiakhq kodiakhq bot deleted the split_source_test branch December 18, 2022 07:44
@cq-bot cq-bot mentioned this pull request Dec 18, 2022
Merged
kodiakhq bot pushed a commit that referenced this pull request Dec 19, 2022
@cq-bot
chore(main): Release v1.12.6 (#506)
74c6672 
🤖 I have created a release *beep* *boop*
---


## [1.12.6](v1.12.5...v1.12.6) (2022-12-18)


### Bug Fixes

* Add better logging/metric per table ([#513](#513)) ([da36396](da36396))
* Improve formatting of newlines in markdown files ([#492](#492)) ([e48ff90](e48ff90))
* Include table name in logs on panic ([#505](#505)) ([a0b8a46](a0b8a46))
* Move source & destination plugin code to separate packages ([#516](#516)) ([6733785](6733785))
* Use correct error codes ([#514](#514)) ([8b53d76](8b53d76))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@erezrokah erezrokah mentioned this pull request Dec 19, 2022
Merged
5 tasks
kodiakhq bot pushed a commit that referenced this pull request Dec 19, 2022
@erezrokah
fix(destination): Rename NewDestinationPlugin to NewPlugin (#519)
3934775 


Follow up to #516

---
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erezrokah erezrokah erezrokah approved these changes

@hermanschaaf hermanschaaf Awaiting requested review from hermanschaaf

Assignees

No one assigned

Labels

automerge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yevgenypats @candiduslynx @erezrokah