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

Add infra for lettuce examples #875

Merged
merged 3 commits into from
Nov 21, 2024

Conversation

uglide
Copy link
Contributor

@uglide uglide commented Nov 13, 2024

No description provided.

In Lettuce we have multiple APIs (async, reactive, etc) and should provide examples for each API variant. To achieve it multiple changes were introduced:
- Add "label" attribute in clients configs and refer to the client example by label instead of language
- Remove hardcoded mapping Label -> Lang from the config.toml
- Prefix copied examples with client name to allow identical naming
- Add configs for Lettuce Async and Reactive APIs
@CLAassistant
Copy link

CLAassistant commented Nov 13, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@andy-stark-redis andy-stark-redis left a comment

Choose a reason for hiding this comment

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

Couple of small fixes/suggestions but otherwise looking great, thanks :-)

config.toml Outdated Show resolved Hide resolved
},
"examples": {
"git_uri": "https://github.com/redis/lettuce",
"dev_branch": "doctests",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to have the examples folder in the main branch? We've tried to do that with the other client libs so that errors against the latest build can be spotted quickly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used the doctests branch just to make changes to the docs testable. As soon as the PR in lettuce is merged, it can be pointed to the main branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's excellent, thanks. Great idea :-)

},
"examples": {
"git_uri": "https://github.com/redis/lettuce",
"dev_branch": "doctests",
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as for async above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See my reply above. Thanks

Co-authored-by: andy-stark-redis <[email protected]>
Copy link
Contributor

@andy-stark-redis andy-stark-redis left a comment

Choose a reason for hiding this comment

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

Approved

@dwdougherty dwdougherty removed their request for review November 20, 2024 16:48
@dwdougherty
Copy link
Collaborator

If @andy-stark-redis is satisfied, then so am I. :)

@andy-stark-redis andy-stark-redis merged commit 25cfd40 into redis:main Nov 21, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants