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

[DEX-258] feat/algolia index config import #79

Merged

Conversation

loicsay
Copy link
Contributor

@loicsay loicsay commented Oct 27, 2022

No description provided.

@loicsay loicsay force-pushed the feat/algolia-index-config-import branch from 750ff75 to 00bb685 Compare October 27, 2022 12:32
@loicsay loicsay changed the title feat/algolia index config import [DEX-258] feat/algolia index config import Oct 27, 2022
@loicsay loicsay force-pushed the feat/algolia-index-config-import branch from f347269 to ebb5c0a Compare October 28, 2022 16:01
@loicsay loicsay self-assigned this Oct 28, 2022
@loicsay loicsay added the enhancement New feature or request label Oct 28, 2022
@loicsay loicsay force-pushed the feat/algolia-index-config-import branch from ebb5c0a to 4a41112 Compare October 28, 2022 16:02
@loicsay loicsay marked this pull request as ready for review October 28, 2022 16:10
@loicsay loicsay force-pushed the feat/algolia-index-config-import branch from 4a41112 to ae0ae1a Compare October 28, 2022 16:13
@loicsay loicsay changed the base branch from main to feat/index-config-import-export October 28, 2022 16:18
@loicsay loicsay marked this pull request as draft November 3, 2022 10:31
@loicsay loicsay force-pushed the feat/algolia-index-config-import branch 4 times, most recently from 2d9e79a to d9ce71b Compare November 7, 2022 17:48
@loicsay loicsay marked this pull request as ready for review November 7, 2022 17:55
@loicsay loicsay requested review from clemfromspace and removed request for clemfromspace November 7, 2022 17:55
@loicsay loicsay force-pushed the feat/algolia-index-config-import branch from d9ce71b to 42a609d Compare November 7, 2022 17:58
@@ -0,0 +1,134 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@clemfromspace
From what I understood, this will never be in the final build. However, I'm not sure this is the best practice to test that feature: wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO, it will be cleaner if you put this file under a sub-directoy like test_artifacts or something similar.

@loicsay loicsay force-pushed the feat/index-config-import-export branch from 601aeaa to a31dc0c Compare November 8, 2022 10:35
@loicsay loicsay force-pushed the feat/algolia-index-config-import branch from 01da3cf to 1308cd2 Compare November 8, 2022 10:36
Copy link
Contributor

@clemfromspace clemfromspace left a comment

Choose a reason for hiding this comment

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

First pass done :)

pkg/cmd/indices/config/import/import.go Show resolved Hide resolved
pkg/cmd/indices/config/import/import.go Outdated Show resolved Hide resolved
pkg/cmd/indices/config/import/import.go Outdated Show resolved Hide resolved
pkg/cmd/indices/config/import/import.go Outdated Show resolved Hide resolved
pkg/cmd/indices/config/import/import.go Outdated Show resolved Hide resolved
pkg/cmd/indices/config/import/import.go Outdated Show resolved Hide resolved
pkg/cmd/indices/config/import/import.go Outdated Show resolved Hide resolved
pkg/cmd/indices/config/import/import.go Show resolved Hide resolved
pkg/cmd/indices/config/import/import.go Outdated Show resolved Hide resolved
pkg/cmd/shared/handler/indices/config.go Show resolved Hide resolved
@loicsay loicsay force-pushed the feat/algolia-index-config-import branch from fa129a3 to e6b0711 Compare November 14, 2022 13:22
Copy link
Contributor

@clemfromspace clemfromspace left a comment

Choose a reason for hiding this comment

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

One last comment, feels free to merge afterwards 👍

pkg/cmd/shared/handler/indices/config.go Outdated Show resolved Hide resolved
@@ -0,0 +1,134 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO, it will be cleaner if you put this file under a sub-directoy like test_artifacts or something similar.

@loicsay loicsay merged commit 4cb49b3 into feat/index-config-import-export Nov 16, 2022
@clemfromspace clemfromspace deleted the feat/algolia-index-config-import branch January 13, 2023 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants