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 package id parsing and avoid NPE when feed is missing some propertie... #776

Merged
merged 4 commits into from
Apr 30, 2015

Conversation

chillitom
Copy link
Contributor

...s

Nuget feed's <m:properties> doesn't contain a <d:Id>, the id is instead found in the 's <title> element.

The element in a nuget feed contains something like "http://www.nuget.org/api/v2/Packages(Id='_Atrico.Lib.CommonAssemblyInfo',Version='1.0.0.0')" which is not what we want here.

Some nuget package sources omit some elements, for example ProGet doesn't report the Language element for handle missing elements by returning an empty string.

…ties

Nuget feed's <m:properties> doesn't contain a <d:Id>, the id is instead found in the <entry>'s <title> element.

The <id> element in a nuget feed contains something like "http://www.nuget.org/api/v2/Packages(Id='_Atrico.Lib.CommonAssemblyInfo',Version='1.0.0.0')" which is not what we want here.

Some nuget package sources omit some elements, for example ProGet doesn't report the Language element for handle missing elements by returning an empty string.
@forki
Copy link
Member

forki commented Apr 29, 2015

Hey that sounds good. But we should use id tag whenever it is there

@chillitom
Copy link
Contributor Author

I haven't seen any feeds that contain an d:Id tag, the <id> atom element is not what we want.. the nuget.org stream doesn't appear to have them either.. am I missing something though?

@chillitom
Copy link
Contributor Author

Fyi, just in the process of putting together a testcase

@chillitom
Copy link
Contributor Author

There isn't an F# tests project from what I can see, am I okay to add one?

@forki
Copy link
Member

forki commented Apr 29, 2015

Yep.
On Apr 29, 2015 2:52 PM, "chillitom" [email protected] wrote:

There isn't an F# tests project from what I can see, am I okay to add one?


Reply to this email directly or view it on GitHub
#776 (comment).

@chillitom
Copy link
Contributor Author

Had difficulty getting a new test project to run as part of the main build so have stuck the new test in the FSCheck.Fake project for now.

I'm remain a little confused as to how the code that was there before worked. Have I done something dumb like reference the wrong stream? It appears to line up with the example in getLatestPackage

forki added a commit that referenced this pull request Apr 30, 2015
Fix package id parsing and avoid NPE when feed is missing some propertie...
@forki forki merged commit 186e987 into fsprojects:master Apr 30, 2015
@forki
Copy link
Member

forki commented Apr 30, 2015

thanks for this.

@chillitom chillitom deleted the patch-1 branch April 30, 2015 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants