search

ljw1004 / platform-specific-analyzer

Finds platform- and version-specific code in your UWP apps
MIT License
14 stars 2 forks source link

Update to support analyzing against 14393 #4

Closed brandonh-msft closed 8 years ago

brandonh-msft commented 8 years ago

Lucian,

Check out the changes below. It's a quick first pass based on my understanding of what the code's trying to do and how it's doing it. Since it's in VB and I'm a c# guy I could've easily made some mistakes.

LMK if there's anything that needs to be changed. I documented the source for the work in the code.

ljw1004 commented 8 years ago

Thanks for the PR, @brandonh-msft. What you've done makes sense but I think it's incomplete...

Also, when I wrote it for 10240-10586, it turned out that all of the changes for 10586 were solely contained inside Windows.Foundation.UniversalApiContract, and so I only bothered checking API-by-API differences inside this assembly. Have you checked that the same is true for 10240-14393?

Also, when I did it for 10240-10586, the changes to other contract were limited to solely "if it's in this set of contracts then it's in 10586 otherwise its in 10240" because every contract other than UniversalApiContract still had the exact same version number. Is the same still true of 14393?

Also, are there any other additional contracts in 14393 that need to be checked?

(I'd written the code this way as a sort of hand-coded efficient check, so it tried to avoid doing dictionary lookup unless absolutely necessary. It's only worth preserving this hand-coded efficiency if it's easy to do correctly :) )

brandonh-msft commented 8 years ago

@ljw1004 I think you may have been looking at an older commit somehow - I've genericized the collection lookup and that variable should no longer exist....

I used the diff located at the URL I added as a code comment to write the dictionary contents for 14393. The lookup logic then flows as a "cumulative" comparison; we never compare 14393 to 10240.

I'm not sure what you mean by "any additional contracts" - if you could clarify I'll see what I can come up with. Still waiting on an answer regarding that internal API Diff tool, so I had to go 3rd party with it.

ljw1004 commented 8 years ago

Doh. I'm sorry. I'm not very good at github yet. I see now the final version of the file. I'll approve the PR.

I don't have access to a Windows machine to build the changes. If you can build the solution in Release mode and email me the .nupkg file, I'll be happy to upload it to NuGet.

Additional Contracts. I imagine something like this

For simplicity, let's pretend that no types have migrated from one contract (windows.*.winmd file) to another. Therefore in this toy example...

brandonh-msft commented 8 years ago

@ljw1004 When I build in Release and then run nuget pack on the nupkg folder under the Analyzer project, it complains about a few things: 1) the powershell scripts aren't in the tools subfolder 2) there is no binary present in the resulting nupkg file; should there be? It packages up the .targets file, the .ps1s, and the .png icon, but nothing else

Thoughts? I'm not 100% sure how Analyzers are installed via nuget, so I don't want to take any guesses but am happy to do so :)