Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 18, 2025

Problem

The checkVersions.yml workflow currently hardcodes the Maven property name releaseNumberSDK for determining the stream version used in commit messages. Different projects may need to use different Maven property names for this purpose, making the workflow less flexible.

Solution

This PR adds a new optional input parameter currentStreamVersionProperty to the checkVersions.yml workflow, allowing callers to specify which Maven property to use for determining the stream version.

Changes

  • Added new input parameter: currentStreamVersionProperty with a default value of 'releaseNumberSDK' to maintain backward compatibility
  • Updated Maven command: Changed from hardcoded releaseNumberSDK to use the configurable parameter
  • Generic naming: Updated temporary file names and comments to reflect the configurable nature

Backward Compatibility

This change is fully backward compatible. Existing workflows that call checkVersions.yml (such as pr-checks.yml) will continue to work without any modifications, as the parameter has a sensible default value.

Usage Examples

Default behavior (uses releaseNumberSDK):

uses: ./.github/workflows/checkVersions.yml
with:
  botName: Eclipse Platform Bot
  botMail: [email protected]

Custom property:

uses: ./.github/workflows/checkVersions.yml
with:
  botName: Eclipse Platform Bot
  botMail: [email protected]
  currentStreamVersionProperty: 'myCustomVersionProperty'

Fixes the issue by making the stream version property configurable while maintaining backward compatibility.

Original prompt

Make releaseNumberSDK configurable parameter in checkVersions.yml

Currently we determine the "releaseNumberSDK" to be used in commit messages by a fixed value, but depending on the project this value might vary.

It would be good to make it configurable e.g. as a currentStreamVersionProperty in the workflow call inputs.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Make releaseNumberSDK configurable in checkVersions.yml Make releaseNumberSDK configurable parameter in checkVersions.yml Oct 18, 2025
Copilot AI requested a review from laeubi October 18, 2025 10:16
Copilot finished work on behalf of laeubi October 18, 2025 10:16
Copy link
Contributor

@laeubi laeubi left a comment

Choose a reason for hiding this comment

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

Looks useful as it at least allows to configure things, having a property in the pom also seems flexible enough for most use-cases.

@laeubi laeubi marked this pull request as ready for review October 18, 2025 10:20
Copy link
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

Checking the input is probably not necessary, see my comment below.
Besides that I think this is fine.

Comment on lines 97 to 102
# Validate property name to prevent injection
propertyName="${{ inputs.currentStreamVersionProperty }}"
if [[ ! "$propertyName" =~ ^[a-zA-Z0-9._-]+$ ]]; then
echo "::error::Invalid property name: $propertyName. Must contain only alphanumeric characters, dots, hyphens, and underscores."
exit 1
fi
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this is necessary as this job doesn't have any permissions:

and with extra-setup-command we have another argument that is also executed as it is.
So this extra check does not really buy us extra safety and therefore I suggest to keep it simple and remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot please remove the additional validation as per @HannesWell suggestion in #3418 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the validation check as requested. The workflow now directly uses the input parameter without validation, consistent with how extra-setup-command is handled. (commit: will be in next message)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the validation check as requested. The workflow now directly uses the input parameter without validation, consistent with how extra-setup-command is handled. (commit e88bdc5)

@HannesWell
Copy link
Member

@laeubi how should we proceed here?

@akurtakov
Copy link
Member

What are the plans here? If no one has intention to finish it maybe we should close it?

@laeubi
Copy link
Contributor

laeubi commented Nov 10, 2025

I would be fine with remove the validation and merge it then.

@laeubi laeubi requested a review from HannesWell November 10, 2025 13:12
Copilot finished work on behalf of laeubi November 10, 2025 13:13
Copilot AI requested a review from laeubi November 10, 2025 13:13
@laeubi
Copy link
Contributor

laeubi commented Nov 10, 2025

@HannesWell if you are fine with it I'll squash/rebase (or you can do it on your self of course)

@HannesWell HannesWell force-pushed the copilot/make-releasenumber-sdk-configurable branch 3 times, most recently from 55af03b to 682960c Compare November 10, 2025 18:03
@HannesWell HannesWell force-pushed the copilot/make-releasenumber-sdk-configurable branch from 682960c to 8440759 Compare November 10, 2025 18:04
Copy link
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

if you are fine with it I'll squash/rebase (or you can do it on your self of course)

Done it.
I also unified and shortened the variable name a bit to be consistent with the other inputs (lower in the list, the style is already inconsistent :/ ).

@HannesWell HannesWell force-pushed the copilot/make-releasenumber-sdk-configurable branch from dc8ee77 to 8440759 Compare November 10, 2025 18:17
@HannesWell HannesWell merged commit b821af1 into master Nov 10, 2025
11 checks passed
@HannesWell HannesWell deleted the copilot/make-releasenumber-sdk-configurable branch November 10, 2025 18:54
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.

4 participants