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

Updating icon package #30

Merged
merged 2 commits into from
Mar 16, 2017
Merged

Updating icon package #30

merged 2 commits into from
Mar 16, 2017

Conversation

eirannejad
Copy link
Contributor

@eirannejad eirannejad commented Mar 15, 2017

Also added exception handling to 6de4f46

IMPORTANT: Credit https://icons8.com/ for the icons.

Added Icon for RevitLookup button in Revit UI
Added icon to RevitLookup forms
Revised Icons for RevitLookup menu bar
Path.GetDirectoryName would through a System.ArgumentException if the assembly.Location is null.
@jeremytammik
Copy link
Owner

hi ehsan, thank you very much for the update! regarding crediting icons8.com, do you think it will be enough to mention that in the repository readme? or where else would you suggest adding it?

@jeremytammik jeremytammik merged commit 21a8ff5 into jeremytammik:master Mar 16, 2017
jeremytammik pushed a commit that referenced this pull request Mar 16, 2017
@eirannejad
Copy link
Contributor Author

Yes that's what I have done. Thry have a permissive license that needs link back. So a link at the credit section on the repo should be good I think.

@jeremytammik
Copy link
Owner

i added a mention at the end of the readme. do you think that is sufficient?

@eirannejad
Copy link
Contributor Author

Yes. RevitLookup is an opensource project so the website says you'll get it for free. I think the credit link works.
https://icons8.com/license/

@jeremytammik
Copy link
Owner

@jeremytammik
Copy link
Owner

jeremytammik commented Mar 17, 2017

the compilation now generates three warning messages:

C:\a\vs\RevitLookup\CS\Snoop\Forms\BindingMap.cs(93,18,93,37): warning CS0108: 'BindingMap.InitializeComponent()' hides inherited member 'ObjTreeBase.InitializeComponent()'. Use the new keyword if hiding was intended.
C:\a\vs\RevitLookup\CS\Snoop\Forms\Categories.cs(72,18,72,37): warning CS0108: 'Categories.InitializeComponent()' hides inherited member 'ObjTreeBase.InitializeComponent()'. Use the new keyword if hiding was intended.
C:\a\vs\RevitLookup\CS\Snoop\Forms\Geometry.cs(127,18,127,37): warning CS0108: 'Geometry.InitializeComponent()' hides inherited member 'ObjTreeBase.InitializeComponent()'. Use the new keyword if hiding was intended.

would you like to fix those so it compiles with zero warnings again?

or shall i just go ahead and do what the man says?

thank you!

@jeremytammik
Copy link
Owner

please ignore my previous comment... i fixed those warning myself in release 2017.0.0.22.

@eirannejad
Copy link
Contributor Author

Huh! I used the VS 2017 Community and didn't get any warnings! It compiled clean multiple times. That's odd.

@jeremytammik
Copy link
Owner

oh, cool. well, trivially fixed my end too now, so no problem. thank you very much for the icons! did you see alexander through your try-catch was excessive? it should indeed always be used as sparingly as possible.

@eirannejad
Copy link
Contributor Author

Yes I just saw that. I saw something somewhere else that said the try catch is very expensive and should be used sparingly. I'll read about it more and try to fix as much as them as I can in the pyRevit code.

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