Skip to content

Conversation

@15knots
Copy link
Contributor

@15knots 15knots commented Jun 4, 2019

Sync with jenkinsci/cmakebuilder-plugin@2725c65
Sorry for the confusion

Submitter checklist

Desired reviewers

@jenkinsci/chinese-localization-sig

Add\ build\ tool\ invocation=\u6DFB\u52A0\u6784\u5EFA\u5DE5\u5177

CmakeBuilder.build_steps=\u6784\u5EFA\u6B65\u9AA4
CmakeBuilder.actual_build_tool=make, ninja, nmake, ...
Copy link
Member

Choose a reason for hiding this comment

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

Thanks to keeping track this. But there's no good back compatible solution. So I suggest that just keep the old one, then mark them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you will have to keep the old version (Run\ build\ tool=) for backward compatibility and
haave to ADD the new version (CmakeBuilder.build_steps=)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please also note that my pull request actually uses
build_steps= and
actual_build_tool= for the property names.
The prefix CmakeBuilder. has been removed since it is already in the package name.

Copy link
Member

Choose a reason for hiding this comment

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

If I understand correctly, we should keep CmakeBuilder.build_steps instead of removing them. Right? Plus, mark the old ones as stale.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Keep both, marke the old ones as stale.

@LinuxSuRen LinuxSuRen merged commit 7b943c4 into jenkinsci:master Sep 10, 2019
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