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 to view export 2018 #1924

Merged
merged 4 commits into from Sep 18, 2018
Merged

fix to view export 2018 #1924

merged 4 commits into from Sep 18, 2018

Conversation

ksobon
Copy link
Contributor

@ksobon ksobon commented Jan 27, 2018

Purpose

This addresses this issue: DynamoDS/Dynamo#3262 (possibly but it's an old one) and this issue: #1695

Basically the idea is that we use ImageExportOptions.GetFileName instead of manually constructing the predicted filename that Revit will assign to our export. It fails.

image

Using their built in static method does not:

image

Declarations

Check these if you believe they are true

  • The code base is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • Snapshot of UI changes, if any.

Reviewers

@mjkkirschner

FYIs

I am re-submitting this since last time it was closed without getting merged. My bad.

@mjkkirschner
Copy link
Member

@ksobon it's likely that some of our tests fail when run with these changes - did you ever get those running with RTF locally?

@mjkkirschner
Copy link
Member

whoops sorry, didn't mean to close.

@mjkkirschner mjkkirschner reopened this Feb 19, 2018
@ksobon
Copy link
Contributor Author

ksobon commented Feb 21, 2018

@mjkkirschner never got the RTF running on my end. What are the issues? Why is it failing?

@ksobon
Copy link
Contributor Author

ksobon commented Mar 14, 2018

@mjkkirschner poke

@ksobon
Copy link
Contributor Author

ksobon commented Apr 11, 2018

@mjkkirschner I finally got the RTF setup so i am able to run the tests myself. You were right this one was failing due to some logic issues and variable conflation. Anyways, it's fixed now:

image

I will cherry pick this to other branches as well. It looks like you guys are busy and cherry picks turn into turf wars. :-)

@mjkkirschner
Copy link
Member

@ksobon nice! Glad you were able to get RTF setup.

@ksobon
Copy link
Contributor Author

ksobon commented Apr 11, 2018

@mjkkirschner well to be honest it's still touchy as hell but it's working so i can't complain. 👍

@ksobon
Copy link
Contributor Author

ksobon commented Apr 11, 2018

@mjkkirschner one more thing. 2019 branch wants me to install .NET 4.7 are you guys running that build from Visual Studio 2017? or should I just install the dev pack and continue working in VS15? I just want to make sure I have a similar setup to avoid issues.

@ksobon ksobon changed the title fix to view export fix to view export 2018 Apr 11, 2018
@QilongTang
Copy link
Contributor

@ksobon Most of our devs here are using VS15 with .Net4.7 dev pack, you can stick to this setup. If you are running VS17 that's also fine

@ZiyunShang
Copy link
Collaborator

I tried to run the RTF test, and it looks good.

@ZiyunShang ZiyunShang merged commit d145a43 into DynamoDS:Revit2018 Sep 18, 2018
AndyDu1985 added a commit that referenced this pull request Sep 20, 2018
Cherry-Pick exportImage from pull request #1924 , ksobon/exportImage
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.

None yet

4 participants