spring-batch icon indicating copy to clipboard operation
spring-batch copied to clipboard

String array command with SystemCommandTasklet

Open philippe-tr opened this issue 4 years ago • 5 comments
trafficstars

Thank you for taking time to contribute this pull request! You might have already read the contributor guide, but as a reminder, please make sure to:

  • Sign the contributor license agreement
  • Rebase your changes on the latest master branch and squash your commits
  • Add/Update unit tests as needed
  • Run a build and make sure all tests pass prior to submission

For more details, please check the contributor guide. Thank you upfront!

philippe-tr avatar Jul 26 '21 19:07 philippe-tr

@philippe-tr Thank you for your PR! Could you please rebase it on the latest main branch? It would be easier for us to only review the changes related to the PR. Thank you upfront.

fmbenhassine avatar Aug 05 '21 12:08 fmbenhassine

PR updated to latest main.

philippe-tr avatar Aug 06 '21 15:08 philippe-tr

Hi. Will this make into 4.2.8 or 4.3.4, or 5.0.0?

philippe-tr avatar Sep 01 '21 15:09 philippe-tr

This is better merged in v5 along with #3972.

As a side note, it seems that you did a merge and not a rebase. I will try to take care of this when tackling this PR.

fmbenhassine avatar Sep 02 '21 12:09 fmbenhassine

@benas Thanks for handling for correcting my merge to a rebase. I got tripped up with the upstream, forked, and local repos. I'll be more careful next time.

philippe-tr avatar Dec 15 '21 00:12 philippe-tr

Rebased, squashed and merged as 746c9193725806a2194042a7e5be063dceca5e1a. The feature of passing a single String for the command and its arguments turned out to be confusing, so it was not merged. The way to specify the command and its arguments is through an array of Strings (similar to the ProcessBuilder API), which is less confusing and makes the implementation simpler (see refinement in 26c39cbbae55cbc0b7b0b61bea4091f26dc19666).

Thank you for your contribution!

fmbenhassine avatar Oct 12 '22 15:10 fmbenhassine