Skip to content

Added logic for working with Tarantool schema via Box #426

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KaymeKaydex
Copy link

@KaymeKaydex KaymeKaydex commented Dec 27, 2024

  • Implemented the box.Schema() method that returns a Schema object for schema-related operations

What has been done? Why? What problem is being solved?

I didn't forget about (remove if it is not applicable):

Related issues:

@KaymeKaydex KaymeKaydex force-pushed the box/schema branch 2 times, most recently from 2e271da to 9f736f4 Compare December 27, 2024 17:51
Copy link
Collaborator

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

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

Please, add more unit-tests for a new requests/response. You could see how it is tested on the main package and inside the crud subpackage.

We also need examples (in example_test.go) with new features. This will help users to use the library.

Finally, we need to keep a backward compatibility until a next major release, so we need to add another Info() method with a context instead of changing the current one.

@KaymeKaydex KaymeKaydex force-pushed the box/schema branch 11 times, most recently from 5c9bb77 to 1e3625c Compare January 21, 2025 15:15
@KaymeKaydex
Copy link
Author

Please, add more unit-tests for a new requests/response. You could see how it is tested on the main package and inside the crud subpackage.

We also need examples (in example_test.go) with new features. This will help users to use the library.

Finally, we need to keep a backward compatibility until a next major release, so we need to add another Info() method with a context instead of changing the current one.

i removed interface changes
added new examples for new methods
and added more tests to make coverage up, now all methods are covered

@KaymeKaydex
Copy link
Author

Please, fix the commit message:

box: ddded logic for working with Tarantool schema

Implemented the `box.Schema()` method that returns a `Schema`
object for schema-related operations

done

@KaymeKaydex KaymeKaydex force-pushed the box/schema branch 2 times, most recently from 40555ec to 4954fc7 Compare January 30, 2025 09:19
@KaymeKaydex KaymeKaydex closed this Feb 5, 2025
@KaymeKaydex KaymeKaydex reopened this Mar 14, 2025
@KaymeKaydex KaymeKaydex force-pushed the box/schema branch 9 times, most recently from 3695443 to 5d690db Compare April 16, 2025 09:56
Implemented the `box.Schema()` method that returns a `Schema`
object for schema-related operations
@KaymeKaydex
Copy link
Author

@oleg-jukovec oleg-jukovec requested review from dmyger and removed request for DerekBum and themilchenko May 25, 2025 17:48
Copy link
Collaborator

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

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

Thanks for the patch. It would be nice if @dmyger paid attention to the test per function coverage.

As I could see the code is not covered (and some MessagePack unmarshal errors, but it is not critical):
image

Please, add a test for it.

Comment on lines +20 to +24
// Schema returns a new Schema instance, providing access to schema-related operations.
// It uses the connection from the Box instance to communicate with Tarantool.
func (b *Box) Schema() *Schema {
return NewSchema(b.conn)
}
Copy link
Collaborator

@oleg-jukovec oleg-jukovec May 25, 2025

Choose a reason for hiding this comment

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

It would be nice to write a unit-test that checks whether the created circuit uses the connection passed to box.New.

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

Successfully merging this pull request may close these issues.

3 participants