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

Fixes 1218: Welcome Card does not display on non-Windows environments #1223

Merged
merged 1 commit into from
Feb 12, 2019

Conversation

sgellock
Copy link
Member

Fixes #1218

Proposed Changes

  • use Path.Combine to build file path to welcomeCard.json

Testing

Build and run on macOS
Should see a welcome card as soon as Emulator connects to the Bot

- use Path.Combine to build file path to welcomeCard.json
@sgellock sgellock merged commit ad2684d into master Feb 12, 2019
@sgellock sgellock deleted the sgellock/fixes-1218 branch February 12, 2019 02:54
@@ -210,7 +210,9 @@ private Activity CreateResponse(Activity activity, Attachment attachment)
// Load attachment from file.
private Attachment CreateAdaptiveCardAttachment()
{
var adaptiveCard = File.ReadAllText(@".\Dialogs\Welcome\Resources\welcomeCard.json");
string[] paths = {".", "Dialogs", "Welcome", "Resources", "welcomeCard.json"};
Copy link
Member

Choose a reason for hiding this comment

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

@sgellock What is this? A comment explaining why this is necessary would be good here. (this is weird)

@sgellock
Copy link
Member Author

sgellock commented Feb 12, 2019 via email

@EricDahlvang
Copy link
Member

Windows + Azure.

@sgellock
Copy link
Member Author

I don't understand your comment. You can't simply hard code file paths in source code that is expected to run on OS's other than windows. e.g. macOS for local development, Linux distros for deployment. The code as is simply doesn't run on non-windows OSs. what additional information do you need to understand the code change?

@EricDahlvang
Copy link
Member

EricDahlvang commented Feb 12, 2019

Sorry, my 'Windows + Azure' comment was facetious.

In seriousness, I've just never seen the syntax here. A requirement of concatenating folder names to retrieve a file at run-time. Maybe I'm just ignorant of this required method of cross-platform path resolution and it will be obvious to more informed developers.

@sgellock
Copy link
Member Author

@EricDahlvang
Copy link
Member

Thanks Scott. Apparently, I'm not keeping up well enough with the cross-platform world. :/

Here's a PR for some of the other samples:
@sgellock #1224

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