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

Use spring-cloud-function-dependencies in time-spel-log #131

Closed
wants to merge 1 commit into from

Conversation

onobc
Copy link
Collaborator

@onobc onobc commented Jan 17, 2025

This replaces the use of spring-cloud-dependencies in time-spel-log with spring-cloud-function-dependencies in order to:

This replaces the use of `spring-cloud-dependencies` in time-spel-log
with `spring-cloud-function-dependencies` in order to:

- narrow dependencies to only what the sample needs

- pick up spring-cloud/spring-cloud-function#1224 which will be
  released earlier than Spring Cloud `2024.0.1` in Spring Cloud Function
  `4.2.1`

Signed-off-by: Chris Bono <[email protected]>
@onobc onobc requested a review from artembilan January 17, 2025 03:02
@artembilan
Copy link
Member

See DCO failure, first.
And second: I don’t see too much problem to stick with SC snapshot for time being . In the end this is just a sample.

And there is another argument. Since this is indeed a sample , it would be better to expose as less discrepancy as possible. Consistency is to include SC deps management as it is suggested by start.spring.io.

Do you have some arguments to justify your change?

My apology if this late night language sounds rude a bit 😅. I know how it is painful when your contribution is rejected 🙇‍♂️

@onobc
Copy link
Collaborator Author

onobc commented Jan 17, 2025

See DCO failure

Yep. Will remove the "Signed off" and GPG sign the commit.

And second: I don’t see too much problem to stick with SC snapshot for time being . In the end this is just a sample.

And there is another argument. Since this is indeed a sample , it would be better to expose as less discrepancy as possible. Consistency is to include SC deps management as it is suggested by start.spring.io.

Do you have some arguments to justify your change?

The arguments are in the PR description. My thinking was that we would not get the fix in 2024.0.1 in time. However, this is a sample and is not released so we do not need to worry about overriding w/ the early 4.2.1 SCF release. Based on this, there is no need for this change and am closing now. Good catch!

My apology if this late night language sounds rude a bit 😅. I know how it is painful when your contribution is rejected 🙇‍♂️

No worries my friend. 1) It was not rude and 2) getting rejected is part of this process 😸

@onobc onobc closed this Jan 17, 2025
@onobc onobc deleted the use-scf-dep-in-time-spel-log branch January 17, 2025 16:57
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.

2 participants