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

Make redis-rs part of this repo #2456

Merged
merged 3 commits into from
Oct 15, 2024

Conversation

eifrah-aws
Copy link
Contributor

@eifrah-aws eifrah-aws commented Oct 14, 2024

In addition to the above:

  • Provide a Makefile for building all bindings
  • Improved python DEVELOPER.md file
  • Small fix to python.yml using the proper solution for fixing installation of tools

With the new Makefile, one can run:

make java

In order to build, lint & test java:

make java java-lint java-test

This is also supported for python, node and go

To get a full list of possible Makefile targets:

make help

Linked issue: #2450

Signed-off-by: Eran Ifrah <eifrah@amazon.com>
@eifrah-aws eifrah-aws marked this pull request as ready for review October 14, 2024 17:54
@eifrah-aws eifrah-aws requested a review from a team as a code owner October 14, 2024 17:54
- Separate the "test" target from the "lint"
- Added new "help" target for listing all possible targets

Signed-off-by: Eran Ifrah <eifrah@amazon.com>
Signed-off-by: Eran Ifrah <eifrah@amazon.com>
Copy link
Collaborator

@avifenesh avifenesh left a comment

Choose a reason for hiding this comment

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

I guess at this point, we're skipping the work on the package itself to strip it from unneeded parts? I should ignore the package, itself?
Can we have an issue statin the need to strip it with a release tag on it? (1.3 maybe?)

.github/workflows/semgrep.yml Show resolved Hide resolved
glide-core/Cargo.toml Show resolved Hide resolved
glide-core/redis-rs/Makefile Show resolved Hide resolved
glide-core/redis-rs/Makefile Show resolved Hide resolved
@eifrah-aws eifrah-aws merged commit 9d953d3 into valkey-io:release-1.2 Oct 15, 2024
37 checks passed
Muhammad-awawdi-amazon pushed a commit that referenced this pull request Oct 15, 2024
* Make redis-rs part of this repo
* Improved PYTHON DEVELOPERS.md file
* Added Makefile for unified build method for all the languages
Muhammad-awawdi-amazon pushed a commit to Muhammad-awawdi-amazon/valkey-glide that referenced this pull request Oct 15, 2024
* Make redis-rs part of this repo
* Improved PYTHON DEVELOPERS.md file
* Added Makefile for unified build method for all the languages

Signed-off-by: Muhammad Awawdi <Mawawdi@amazon.com>
Copy link
Collaborator

@Yury-Fridlyand Yury-Fridlyand left a comment

Choose a reason for hiding this comment

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

Changelog is missing

PY_PATH=$(shell find python/.env -name "site-packages"|xargs readlink -f)
PY_GLIDE_PATH=$(shell pwd)/python/python/

all: java java-test python python-test node node-test go go-test python-lint java-lint
Copy link
Collaborator

Choose a reason for hiding this comment

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

node-lint go-lint are missing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Already added them in later commit

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean those are are not parts of all.
BTW, do you want to add tasks for c# client too?

Copy link
Collaborator

Choose a reason for hiding this comment

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

There are no tasks for testing server modules

Makefile Show resolved Hide resolved
@@ -105,7 +105,7 @@ Before starting this step, make sure you've installed all software requirements.
git clone --branch ${VERSION} https://github.com/valkey-io/valkey-glide.git
cd valkey-glide
```
2. Initialize git submodule:
2. Initialize git submodules:
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this from all docs - we don't need this anymore

@@ -163,7 +163,7 @@ go test -race ./... -run TestConnectionRequestProtobufGeneration_allFieldsSet -v
After pulling new changes, ensure that you update the submodules by running the following command:

```bash
git submodule update
git submodule update --init --recursive
Copy link
Collaborator

Choose a reason for hiding this comment

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

same

@@ -1,4 +1,5 @@
# Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0
# mypy: disable_error_code="arg-type"
Copy link
Collaborator

Choose a reason for hiding this comment

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

why?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider renaming the dir and updating readme.

which redis-server

clean:
rm -fr .build/
Copy link
Collaborator

Choose a reason for hiding this comment

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

include cleaning for all clients + core there

avifenesh pushed a commit to avifenesh/valkey-glide that referenced this pull request Oct 21, 2024
* Make redis-rs part of this repo
* Improved PYTHON DEVELOPERS.md file
* Added Makefile for unified build method for all the languages
avifenesh pushed a commit to avifenesh/valkey-glide that referenced this pull request Oct 21, 2024
* Make redis-rs part of this repo
* Improved PYTHON DEVELOPERS.md file
* Added Makefile for unified build method for all the languages
Muhammad-awawdi-amazon pushed a commit to Muhammad-awawdi-amazon/valkey-glide that referenced this pull request Oct 22, 2024
* Make redis-rs part of this repo
* Improved PYTHON DEVELOPERS.md file
* Added Makefile for unified build method for all the languages
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