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.
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
Add removeArtifacts #109
Add removeArtifacts #109
Conversation
Hey @Rosalie241, |
Done, tests should pass now, though I wasn't sure how to exactly add/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good, we just need to update the ArtifactUploader
test now. Here are the steps I'd take to update that test:
- Add a new argument to the test method which creates the artifact upload -
removes
. This method can be found herefunction createUploader(replaces: boolean, throws: boolean = false): GithubArtifactUploader { - Pass this new argument into the constructor for ArtifactUploader in the test.
- Copy the
replaces all artifacts
test. Name the new testremoves all artifacts
. That test can be found here:it('replaces all artifacts', async () => { - Update the new test so it sets your flag instead of the
replaces
flag. I think you can leave the body of the test mostly the same. This will just verify that the uploader will delete all of the existing artifacts.
added & pushed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks for the PR!! I'll do some additional testing next week then cut a new release with these changes in it.
it seems like this works fine though I'm still not experienced with typescript/javascript so feedback is welcome.
Closes #108