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.

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

Fix for #148 #232

Merged
merged 1 commit into from
Feb 25, 2020
Merged

Fix for #148 #232

merged 1 commit into from
Feb 25, 2020

Conversation

markwragg
Copy link
Contributor

@markwragg markwragg commented Jan 23, 2020

This fixes a bug in Get-VSTeamBuildArtifact where an error is returned because the API returns an extra record along with the list of artifacts that is called 'build.sourcelabel' and contains a URL but no "properties" object.

This PR stops the error by excluding any build artifact records returned by the API that don't contain a "properties" object from getting the build artifact specific type names applied. The result is still returned to the user however.

Alternatively if we think it would be better to exclude this result entirely, then it could instead be filtered out in the Get-VSTeamBuildArtifact function.

PR Summary

PR Checklist

This fixes a bug in `Get-VSTeamBuildArtifact` where an error is returned because the API returns an extra record along with the list of artifacts that is called 'build.sourcelabel' and contains a URL but no "properties" object.

This PR filters out this result by excluding any build artifact records returned by the API that don't contain a "properties" object.

Alternatively if we think its desirable to include this artifact in the result, we could just wrap an `if` statement around the line of code that attempts to add a typename to the `properties` property, so that it doesn't attempt to do this on the one artifact where it doesn't exist.
@SebastianSchuetze
Copy link
Collaborator

SebastianSchuetze commented Jan 24, 2020

Hey @markwragg . I am not the maintainer, but maybe it could be good to add a unit test to exactly for the problem you try to fix? Would that be possible?

@DarqueWarrior DarqueWarrior added the Investigating when an issue needs further investigation by a maintainer. Used only by maintainer. label Feb 11, 2020
@SebastianSchuetze
Copy link
Collaborator

@markwragg would you able to add a unit test for this? Do you need help? I am also now a maintainer of this repo. So I could help you much better with problems regarding your PR.

@MichelZ
Copy link
Contributor

MichelZ commented Feb 24, 2020

I've added a test for this in my PR #257
Feel free to copy it into your PR and to merge this one :)

@DarqueWarrior DarqueWarrior merged commit f84dc38 into MethodsAndPractices:master Feb 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Investigating when an issue needs further investigation by a maintainer. Used only by maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants