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

Updated crux-xodus to xtdb-xodus #6

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

vamem9z-Moses
Copy link

I've updated the crux xodus to use adapter to use the latest version of xtdb (the rename of crux) and the latest version of xodus. I suggest renaming it to xtdb-xodus to match xtdb and have done the work necessary to do so. Let me know what you think.

Copy link
Collaborator

@mitchelkuijpers mitchelkuijpers left a comment

Choose a reason for hiding this comment

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

Thank you so much for this PR as you probably figured we are not using this extensively anymore. I only have a few small remarks and then I think we can merge it. I would also be happy to do a rename but I would like to discuss this with the XTDB team first so I don't break any links.

Comment on lines 1 to 3
{
"java.configuration.updateBuildConfiguration": "interactive"
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove this and put in the gitignore

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for responding. I'm happy to maintain this adapter if your team is not using it anymore. I am curious as to why you are not and what you switched to as obviously I'm planning on using it for a project. I thought it was appropriate to change the name but I'm happy to change it back if it proves problematic. I've made the changes you've requested see the latest commit.

.gitignore Outdated
Comment on lines 16 to 17
.lsp/.cache
.calva/output-window/output.calva-repl
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be defined in your own .gitignore

Copy link
Author

Choose a reason for hiding this comment

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

Done see latest commit and earlier response.

@vamem9z-Moses
Copy link
Author

Just checking in. Have you had a chance to talk to the xtdb team?

@refset
Copy link

refset commented Jan 12, 2023

Hi :)

Have you had a chance to talk to the xtdb team?

I just spotted this! We've not had a conversation but I see no harm in renaming, links should redirect automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants