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 dependencies #112

Merged
merged 7 commits into from
Aug 31, 2022
Merged

Updated dependencies #112

merged 7 commits into from
Aug 31, 2022

Conversation

alexsnaps
Copy link
Member

@alexsnaps alexsnaps commented Aug 30, 2022

  • froze notify crate
  • updated all deps requiring no code changes
  • update homepage to kuadrant.io
  • updated to latest rustc toolchain
  • removed libssl3 from container image, as this was introduced by reqwest/infinispan
  • updated authors… but it isn't complete… confused?
    • Do we want active authors or all authors… I'd argue for the latter!

@alexsnaps alexsnaps requested a review from eguzki August 30, 2022 13:48
@alexsnaps alexsnaps self-assigned this Aug 30, 2022
@alexsnaps alexsnaps added this to the Limitador Server v1.0.0 milestone Aug 30, 2022
@alexsnaps alexsnaps marked this pull request as draft August 30, 2022 14:09
Copy link
Contributor

@eguzki eguzki left a comment

Choose a reason for hiding this comment

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

LGTM so far

@@ -6,7 +6,7 @@ license = "Apache-2.0"
keywords = ["rate-limiting", "rate", "limiter", "envoy", "rls"]
categories = ["web-programming"]
description = "Rate limiting service that integrates with Envoy's RLS protocol"
homepage = "https://github.com/kuadrant/limitador"
homepage = "https://kuadrant.io"
Copy link
Contributor

Choose a reason for hiding this comment

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

If the limitador-server crate is not being published, where is this information being exposed? (in addition to in the Cargo.toml in the GH repo)

Anyway, for me the homepage is (was) correct. Maybe in the "READMEwe should add reference tohttps://kuadrant.io`. Not a strong opinion, though.

Copy link
Member Author

@alexsnaps alexsnaps Aug 30, 2022

Choose a reason for hiding this comment

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

Which ever… we can also not have a homepage (also, to be clear, there is repository that points to github still)

Copy link
Contributor

Choose a reason for hiding this comment

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

🤷 your call

license = "Apache-2.0"
keywords = ["rate-limiting", "rate", "limiter"]
categories = ["web-programming"]
description = "Rate limiting library"
homepage = "https://github.com/kuadrant/limitador"
homepage = "https://kuadrant.io"
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure about this, since this crate is publish in crates.io

Aligned with the previous comment, I would leave it as it was, but it is ok as you propose.

@@ -6,7 +6,7 @@ license = "Apache-2.0"
keywords = ["rate-limiting", "rate", "limiter", "envoy", "rls"]
categories = ["web-programming"]
description = "Rate limiting service that integrates with Envoy's RLS protocol"
homepage = "https://github.com/kuadrant/limitador"
homepage = "https://kuadrant.io"
repository = "https://github.com/kuadrant/limitador"
documentation = "https://docs.rs/limitador"
Copy link
Member Author

Choose a reason for hiding this comment

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

I'd probably get rid of the documentation here, or have it point to kuadrant.io/docs/limitador-server? (There will be better content there soon™)

@alexsnaps alexsnaps marked this pull request as ready for review August 30, 2022 16:51
@@ -22,6 +22,9 @@ jobs:
profile: minimal
toolchain: stable
override: true
- uses: abelfodil/protoc-action@v1
Copy link
Contributor

Choose a reason for hiding this comment

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

just for curiosity, why is this new step needed now?

Copy link
Member Author

Choose a reason for hiding this comment

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

@alexsnaps alexsnaps merged commit e8cb4b5 into main Aug 31, 2022
@alexsnaps alexsnaps deleted the deps-up branch August 31, 2022 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants