Skip to content

Improve readability of CloudFoundryVcapEnvironmentPostProcessor by removing constants #45855

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Improve readability of CloudFoundryVcapEnvironmentPostProcessor by removing constants #45855

wants to merge 1 commit into from

Conversation

wonyongg
Copy link
Contributor

@wonyongg wonyongg commented Jun 8, 2025

This pull request extracts the hardcoded "vcap" string into a named constant
(VCAP_PROPERTY_SOURCE_NAME) within the CloudFoundryVcapEnvironmentPostProcessor class.

  • Improves readability and consistency
  • Makes the purpose of the property source name clearer
  • Aligns with common practices for well-known property source identifiers

This is a minor internal refactoring and does not affect runtime behaviour.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 8, 2025
@wilkinsona
Copy link
Member

Thanks for the proposal.

The use of constants in this class is certainly a bit inconsistent at the moment. For example, we have VCAP_APPLICATION and VCAP_SERVICES constants that are only used once but with the values repeated in string literal error messages. Then, as you've shown here, we have "vcap" that's used twice without a constant.

Flagging for team attention to see which direction we should go in:

  1. Use constants consistently throughout the class
  2. Remove the use of constants

@wilkinsona wilkinsona added the for: team-attention An issue we'd like other members of the team to review label Jun 9, 2025
@wonyongg
Copy link
Contributor Author

wonyongg commented Jun 9, 2025

@wilkinsona

Thanks for the thoughtful feedback. I'm happy to follow whatever direction the team decides.
Please feel free to let me know if you'd like me to update the PR accordingly.

@mhalbritter
Copy link
Contributor

I'd remove both existing constants from CloudFoundryVcapEnvironmentPostProcessor, i don't feel they make the code clearer.

@wilkinsona
Copy link
Member

@wonyongg could you please update your PR to remove the existing constants rather than introducing a new one?

@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed for: team-attention An issue we'd like other members of the team to review labels Jun 12, 2025
@wonyongg
Remove constants from CloudFoundryVcapEnvironmentPostProcessor
2be5ffd 
This commit removes the VCAP_APPLICATION, VCAP_SERVICES, and
VCAP_PROPERTY_SOURCE_NAME constants from the
CloudFoundryVcapEnvironmentPostProcessor class, replacing them with
string literals.

This change simplifies the code and aligns with the team's decision to
avoid using constants in this context.

Signed-off-by: wonyongg <[email protected]>
@wilkinsona wilkinsona changed the title Extract constant for 'vcap' property source name Improve readability of CloudFoundryVcapEnvironmentPostProcessor by removing constants Jun 12, 2025
@wonyongg
Copy link
Contributor Author

@wilkinsona
Thanks for the clarification! I've updated the PR to remove all constants and replace them with string literals, as suggested.

Please have a look when you have time—I'd really appreciate your review. Thank you!

@wilkinsona wilkinsona added type: task A general task and removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels Jun 12, 2025
@wilkinsona wilkinsona added this to the 3.3.x milestone Jun 12, 2025
@mhalbritter mhalbritter self-assigned this Jun 12, 2025
@mhalbritter mhalbritter mentioned this pull request Jun 12, 2025
Closed
mhalbritter pushed a commit that referenced this pull request Jun 12, 2025
@wonyongg @mhalbritter
Remove constants from CloudFoundryVcapEnvironmentPostProcessor
a300d2c 
This commit removes the VCAP_APPLICATION and VCAP_SERVICES constants
from the CloudFoundryVcapEnvironmentPostProcessor class, replacing them
with string literals.

See gh-45855

Signed-off-by: wonyongg <[email protected]>
@mhalbritter
Copy link
Contributor

Thanks @wonyongg !

@mhalbritter mhalbritter modified the milestones: 3.3.x, 3.3.13 Jun 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@mhalbritter mhalbritter

Labels
type: task A general task
Projects
None yet
Milestone
3.3.13
Development

Successfully merging this pull request may close these issues.
4 participants
@wonyongg @wilkinsona @mhalbritter @spring-projects-issues