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

Separate build steps by adding two new builders #72

Merged
merged 23 commits into from
Mar 3, 2020
Merged

Separate build steps by adding two new builders #72

merged 23 commits into from
Mar 3, 2020

Conversation

nbhoski
Copy link
Member

@nbhoski nbhoski commented Feb 19, 2020

This PR includes the following changes.

  • Added new build environment instance named "Add MATLAB to PATH"
    image
  • Matlab root will now be accepted through this.
  • Added two new build steps
    image
  • All new builds for "Run MATLAB test" will automatically get delegated to new builder class
  • "Run MATLAB tests " will now look like below
    image
  • "Run MATLAB Command " will look like below
    image
  • Moved all command construction-related helper methods to a common utility class.
  • Added tests for both new builder
  • updated the existing test for old builders

Things Not included in the review :

  • Refactoring of existing MatlabBuilder class is yet to be done . i shall evaluate the possibility of using new command construct util. i will track this task with an appropriate issue and raise a separate PR for the same.

@nbhoski
Copy link
Member Author

nbhoski commented Feb 19, 2020

The azure check may fail due to static code analyzer issues. I am looking into it.

mcafaro
mcafaro requested changes Feb 19, 2020
View reviewed changes
src/main/java/com/mathworks/ci/CommandConstructUtil.java Outdated Show resolved Hide resolved
src/main/java/com/mathworks/ci/CommandConstructUtil.java Outdated Show resolved Hide resolved
src/main/java/com/mathworks/ci/MatlabScriptBuilder.java Outdated Show resolved Hide resolved
src/main/java/com/mathworks/ci/MatlabTestRunBuilder.java Outdated Show resolved Hide resolved
src/main/java/com/mathworks/ci/MatlabBuildWrapper.java Outdated Show resolved Hide resolved
src/main/java/com/mathworks/ci/MatlabScriptBuilder.java Outdated Show resolved Hide resolved
src/main/resources/com/mathworks/ci/MatlabScriptBuilder/help-matlabCommand.html Outdated Show resolved Hide resolved
src/main/resources/com/mathworks/ci/MatlabScriptBuilder/config.jelly Outdated Show resolved Hide resolved
@nbhoski nbhoski requested a review from mcafaro February 20, 2020 11:48
mw-hrastega
mw-hrastega requested changes Feb 25, 2020
View reviewed changes
Copy link
Member

@mw-hrastega mw-hrastega left a comment

Choose a reason for hiding this comment

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

The two examples need some tweaks.

src/main/resources/com/mathworks/ci/RunMatlabCommandBuilder/help-matlabCommand.html Outdated Show resolved Hide resolved
src/main/resources/com/mathworks/ci/RunMatlabCommandBuilder/help-matlabCommand.html Outdated Show resolved Hide resolved
mcafaro
mcafaro requested changes Feb 26, 2020
View reviewed changes
src/main/java/com/mathworks/ci/AddMatlabToPathBuildWrapper.java Outdated Show resolved Hide resolved
src/main/java/com/mathworks/ci/CommandConstructUtil.java Outdated Show resolved Hide resolved
src/main/java/com/mathworks/ci/CommandConstructUtil.java Outdated Show resolved Hide resolved
src/main/java/com/mathworks/ci/RunMatlabCommandBuilder.java Outdated Show resolved Hide resolved
src/main/java/com/mathworks/ci/RunMatlabTestsBuilder.java Outdated Show resolved Hide resolved
src/main/java/com/mathworks/ci/RunMatlabTestsBuilder.java Outdated Show resolved Hide resolved
src/main/java/com/mathworks/ci/RunMatlabCommandBuilder.java Outdated Show resolved Hide resolved
src/main/java/com/mathworks/ci/RunMatlabTestsBuilder.java Outdated Show resolved Hide resolved
mw-hrastega
mw-hrastega requested changes Feb 28, 2020
View reviewed changes
Copy link
Member

@mw-hrastega mw-hrastega left a comment

Choose a reason for hiding this comment

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

Some changes to the help text for the build environment.

@nbhoski nbhoski requested a review from mw-hrastega March 2, 2020 11:37
@mw-hrastega
Copy link
Member

The changes look fine to me.

@mw-hrastega mw-hrastega closed this Mar 2, 2020
@mw-hrastega mw-hrastega reopened this Mar 2, 2020
mcafaro
mcafaro approved these changes Mar 2, 2020
View reviewed changes
Copy link
Member

@mcafaro mcafaro left a comment

Choose a reason for hiding this comment

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

Some minor stylistic comments. Otherwise, looks good to go.

src/main/resources/com/mathworks/ci/RunMatlabCommandBuilder/help-matlabCommand.html Outdated Show resolved Hide resolved
src/main/resources/com/mathworks/ci/RunMatlabTestsBuilder/help-coberturaChkBx.html Outdated Show resolved Hide resolved
src/main/resources/com/mathworks/ci/RunMatlabTestsBuilder/help-junitChkBx.html Outdated Show resolved Hide resolved
src/main/resources/com/mathworks/ci/RunMatlabTestsBuilder/help-tapChkBx.html Outdated Show resolved Hide resolved
@nbhoski nbhoski merged commit c093ee9 into CI-368-seperate-buildstep-latest Mar 3, 2020
@nbhoski nbhoski deleted the CI-368-add-new-builders branch July 7, 2020 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcafaro mcafaro mcafaro approved these changes

@acampbel acampbel acampbel approved these changes

@mw-akumar mw-akumar mw-akumar approved these changes

@mw-hrastega mw-hrastega mw-hrastega approved these changes

Assignees

@anjanak-dev anjanak-dev

@acampbel acampbel

@mw-hrastega mw-hrastega

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@nbhoski @mw-hrastega @mcafaro @acampbel @mw-akumar @anjanak-dev