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

Refactor appveyor.psm1 to ci.psm1 #8733

Merged
merged 7 commits into from Jan 25, 2019
Merged

Refactor appveyor.psm1 to ci.psm1 #8733

merged 7 commits into from Jan 25, 2019

Conversation

RDIL
Copy link
Contributor

@RDIL RDIL commented Jan 24, 2019 •
edited

PR Summary

AppVeyor removed, so file has been renamed, as well as many AppVeyor references / env vars / etc. have been removed.

PR Context

PR Checklist

TravisEz13
TravisEz13 requested changes Jan 24, 2019
View reviewed changes
Copy link
Member

@TravisEz13 TravisEz13 left a comment

Choose a reason for hiding this comment

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

Thanks a lot.

Just one comment.

tools/ci.psm1 Outdated Show resolved Hide resolved
@TravisEz13
Copy link
Member

This was work we planned. FYI, We also planned on merging travis.ps1 and removing duplicate code (It doesn't need to be done in the same PR).

@TravisEz13 TravisEz13 self-assigned this Jan 24, 2019
PaulHigin
PaulHigin suggested changes Jan 24, 2019
View reviewed changes
tools/ci.psm1 Outdated Show resolved Hide resolved
TravisEz13 and others added 2 commits January 24, 2019 14:39
@TravisEz13 @RDIL
Update tools/ci.psm1
2aaaf0c 
Co-Authored-By: RDIL <jumbocakefdx@gmail.com>
@RDIL
Address feedback
b40661d
@TravisEz13
Copy link
Member

@PaulHigin Please update your review.

@RDIL
Copy link
Contributor Author

RDIL commented Jan 25, 2019

@PaulHigin I fixed the problems, please re-review

@RDIL
Copy link
Contributor Author

RDIL commented Jan 25, 2019

CLA bot is 🅱️roken

@TravisEz13
Copy link
Member

TravisEz13 commented Jan 25, 2019 •
edited

No worries, The CLA bot previously approved this PR.

@RDIL
Copy link
Contributor Author

RDIL commented Jan 25, 2019 •
edited

Windows build now failing for unknown reason. Looks like a file I haven't touched meaning the failure is not an issue with this PR

@TravisEz13
Copy link
Member

We found a test regressed due to an environmental issue.

@TravisEz13
Copy link
Member

merged the fix for the test. I'm going to try to just retry the windows CI. If it fails again with the same error, ask me to rebase the branch with the fix.

@TravisEz13 TravisEz13 merged commit 04a31ff into PowerShell:master Jan 25, 2019
@TravisEz13
Copy link
Member

Used admin rights to merge. CLA did not run on the last commit, but it approved the commit before that.

@TravisEz13
Copy link
Member

Thanks again @RDIL

@RDIL RDIL deleted the patch-3 branch January 26, 2019 00:03
@iSazonov iSazonov added the CL-Tools Indicates that a PR should be marked as a tools change in the Change Log label Feb 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TravisEz13 TravisEz13 TravisEz13 approved these changes

@adityapatwardhan adityapatwardhan adityapatwardhan approved these changes

@PaulHigin PaulHigin PaulHigin approved these changes

@anmenaga anmenaga Awaiting requested review from anmenaga anmenaga is a code owner

@daxian-dbw daxian-dbw Awaiting requested review from daxian-dbw daxian-dbw is a code owner

Assignees

@TravisEz13 TravisEz13

Labels
CL-Tools Indicates that a PR should be marked as a tools change in the Change Log
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@RDIL @TravisEz13 @adityapatwardhan @PaulHigin @iSazonov