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

Revit_Toolkit: Update to Nugets #353

Merged
merged 5 commits into from
Jul 23, 2019
Merged

Conversation

FraserGreenroyd
Copy link
Contributor

@FraserGreenroyd FraserGreenroyd commented Jul 22, 2019

Issues addressed by this PR

Fixes #350

Test files

N/A - existing test scripts to pull/push from 2018 and 2019 Revit

Changelog

  • Changed references to Nuget packages

Additional comments

The RevitAPIIFC one, I could not find a Nuget package for. I might have missed it, so please feel free to suggest one, however, for now, I have done a local referencing within a repo libs folder to allow our integration checks to find/reference it to compile for PRs. Please review this from a functionality and licencing perspective @adecler and @ZiolkowskiJakub

Please do not merge until both @ZiolkowskiJakub and @adecler have had chance to review this

@FraserGreenroyd FraserGreenroyd added the type:external-api-changes Imposed changes, including from dependency across other BHoM repos label Jul 22, 2019
@FraserGreenroyd FraserGreenroyd self-assigned this Jul 22, 2019
@adecler
Copy link
Member

adecler commented Jul 23, 2019

AppVeyor is happy --> I'm happy.
The tests are done on Debug2018 and Debug2019. Should it be done on Debug as well?
Notice that there are quite a few warnings when compiling, and not just about missing XML. It might be worth addressing that at one point.

I'll leave Jakub to check if everything is still working fine on Revit side. If that's the case, I'm happy for you guys to merge.

Copy link
Member

@ZiolkowskiJakub ZiolkowskiJakub left a comment

Choose a reason for hiding this comment

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

Hi @FraserGreenroyd,

I briefly went through the changes and my question is how this implementation deals with different versions of Revit API?

As far as I can see it always refers Revit dlls for Revit 2018 API.

Do we want to download multiple versions of Nuget packages for different versions of Revit API? How would this solution works for "Restore Nuget packages" option?

@FraserGreenroyd
Copy link
Contributor Author

Hi @ZiolkowskiJakub the Nuget packages provide the functionality for both 2018 and 2019 versions of Revit. We are unable to have different Nuget packages for different configurations unless we use a more recent version of Visual Studio which not everyone will have.

I presume you tested this for both 2018 and 2019 as I did? I was able to successfully pull a model from Revit 2018 and Revit 2019 into Grasshopper and view it/push it to XML (as is my go-to test). If you haven't, give it a try, I think you'll be pleasantly surprised 😄

@ZiolkowskiJakub
Copy link
Member

Hi @ZiolkowskiJakub the Nuget packages provide the functionality for both 2018 and 2019 versions of Revit. We are unable to have different Nuget packages for different configurations unless we use a more recent version of Visual Studio which not everyone will have.

I presume you tested this for both 2018 and 2019 as I did? I was able to successfully pull a model from Revit 2018 and Revit 2019 into Grasshopper and view it/push it to XML (as is my go-to test). If you haven't, give it a try, I think you'll be pleasantly surprised 😄

Hi @FraserGreenroyd ,

Testing it on existing solution is one thing and developing it further is another. Currently we did not touch functionalities which differ much between Revit APIs but I strongly believe we will reach it soon. Example: Autodesk have added many new methods for Materials resources in Revit API 2019 which does not exist in Revit 2018 API, UI selection methods in Revit API 2019 behaves differently than in Revit 2018 etc.. Compiling existing code is one thing and make sure it works correct for specific Revit version is another.

Nuget packages provide the functionality for both 2018 and 2019

Good to hear. Can you tell a little bit more about it? How we will control that?

I saw that there is many versions of the Nuget package:
image

but the question is how you want to work/deploy Revit_Toolkit with that? Is there any way we can automatically download multiple versions at once? I believe I am missing something here...

@FraserGreenroyd
Copy link
Contributor Author

The other option we have is to put the DLLs we want in the libs folder and include them as part of the repository and distribution, and then have the configs set as we previously did. However, there is a question of licencing and whether Autodesk would be acceptable with that. The Nuget packages have, presumably, already been permitted by Autodesk so allow us to use them.

As I say, we can't have different Nuget configurations the same way we had build configurations from what I can see without using newer versions of Visual Studio. So if you're happy there won't be any licencing issues, we could go the libs route as we did with TAS and keep having the two versions.

Alternatively, if we want to keep with the Nuget line of enquiry, then we could make two different RevitUI projects, one for 2018 and one for 2019 which then target the different Nuget packages as appropriate, but that could cause more problems than it solves. @adecler what are your thoughts?

@ZiolkowskiJakub
Copy link
Member

For reference:

Here is list of API changes for Revit 2019: What's New in the Revit 2019 API

@ZiolkowskiJakub
Copy link
Member

Here is reference to the article why we should not copy local Revit dlls:
Set Copy Local to False

@ZiolkowskiJakub
Copy link
Member

I believe the article above is the reason why there is no official Nuget package for Revit. I would say this is "best practise" and I have deployed couple tools in the past with local copies of dlls... it worked fine but we should be aware of unexpected issues related to this kind of implementation.

@FraserGreenroyd
Copy link
Contributor Author

Following discussion with @adecler in the operations call we might go for a different tact on this in the future. For now, let's review this functionality, check it works for now, and @adecler and I will sort out something different for the Nuget's within the next couple of weeks 😄

@ZiolkowskiJakub
Copy link
Member

@al-fisher or @rwemay can you please confirm you are happy with me to approve and merge this PR?

@rwemay
Copy link
Member

rwemay commented Jul 23, 2019

Hey @ZiolkowskiJakub , I won't have time to review today. Is someone available to test and approve?

@ZiolkowskiJakub
Copy link
Member

Hi @rwemay,

It is not about reviewing the changes it is about idea and issues related to proposed changes:
image

Copy link
Member

@ZiolkowskiJakub ZiolkowskiJakub left a comment

Choose a reason for hiding this comment

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

Agreed with @rwemay so it can be merged

@ZiolkowskiJakub ZiolkowskiJakub merged commit a4044e1 into master Jul 23, 2019
@adecler adecler deleted the Revit_Toolkit-Issue350-Nuget branch July 23, 2019 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:external-api-changes Imposed changes, including from dependency across other BHoM repos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove NuGet references to Revit dlls
4 participants