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.

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

minor: extract maven execution from pitest #11763

Merged
merged 1 commit into from Jun 25, 2022
Merged

minor: extract maven execution from pitest #11763

merged 1 commit into from Jun 25, 2022

Conversation

Vyom-Yadav
Copy link
Member

@Vyom-Yadav Vyom-Yadav commented Jun 24, 2022 •
edited by nrmancuso

From discussion at #11731 (comment) and #11720 (comment):

Extract maven execution from pitest.sh script.

Failed pitest run (for proof): https://github.com/checkstyle/checkstyle/actions/runs/2555422670

@nrmancuso
Copy link
Member

This PR depends on #11762, adding blocked label until then.

@nrmancuso nrmancuso self-assigned this Jun 24, 2022
@Vyom-Yadav Vyom-Yadav force-pushed the extractMavenExecutionFromPitest branch 5 times, most recently from 9cd62a5 to 71ef952 Compare June 24, 2022 12:43
@Vyom-Yadav Vyom-Yadav force-pushed the extractMavenExecutionFromPitest branch from 71ef952 to e50ac7a Compare June 24, 2022 15:12
@Vyom-Yadav
Copy link
Member Author

@nick-mancuso #11762 has been merged, this can be reviewed.

@nrmancuso nrmancuso removed the blocked label Jun 24, 2022
@nrmancuso nrmancuso assigned rnveach and unassigned nrmancuso Jun 24, 2022
@nrmancuso nrmancuso mentioned this pull request Jun 24, 2022
Merged
7 tasks
rnveach
rnveach approved these changes Jun 24, 2022
View reviewed changes
Copy link
Member

@rnveach rnveach left a comment •
edited

Choose a reason for hiding this comment

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

I am hoping this is more temporary and am more ok with this if that is the case, but I am not liking we are losing a simple command to run the pitest process locally and now splitting up with no replacement.

@rnveach rnveach assigned romani and unassigned rnveach Jun 24, 2022
@Vyom-Yadav
Copy link
Member Author

I am hoping this is more temporary and am more ok with this if that is the case, but I am not liking we are losing a simple command to run the pitest process locally and now splitting up with no replacement.

Exactly, that will be a habit breaker too, and generating pit reports first and then evaluating them just adds a step.

@nrmancuso nrmancuso removed the request for review from romani June 25, 2022 10:48
@nrmancuso nrmancuso merged commit e9a3d90 into checkstyle:master Jun 25, 2022
@nrmancuso
Copy link
Member

I am merging this to move project along, @romani please see comments above from maintainers regarding this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rnveach rnveach rnveach approved these changes

@nrmancuso nrmancuso nrmancuso approved these changes

Assignees

@romani romani

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Vyom-Yadav @nrmancuso @rnveach @romani