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 #33 & #32 - building site.min.js and site.min.css for release builds and Append version for included styles #34

Merged
merged 5 commits into from Feb 4, 2019

Conversation

ghost
Copy link

@ghost ghost commented Feb 4, 2019

No description provided.

@ghost ghost changed the title Fix building site.min.js and site.min.css for release builds Fix #33 & #32 - building site.min.js and site.min.css for release builds and Append version for included styles Feb 4, 2019
@maraf
Copy link
Owner

maraf commented Feb 4, 2019

Thanks a lot, @nicotravassos!

Looks good to me, but I would like to discuss one thing:

Is there any benefit in using attribute names on environment instead of include and exclude? When I generate a new ASP.NET Core site from official template, it uses include and exclude.

@ghost
Copy link
Author

ghost commented Feb 4, 2019

Hello @maraf,

names accepts a single hosting environment name or a comma-separated list of hosting environment names that trigger the rendering of the enclosed content.

include & exclude attributes control rendering the enclosed content based on the included or excluded hosting environment names.

Please see this asp-net-core-environment-tag-helper

It seems names was used in ASPNETCORE 1 and include and exclude ASPNETCORE 2 see environment-taghelper-exclude-and-include-attributes-in-asp-net-core-2

You can always exclude it from the pull-request 😄

@maraf
Copy link
Owner

maraf commented Feb 4, 2019

So, please remove the change regarding include/exclude replacement and I'm going to merge it.

Switch back to `include` and `exclude`
I will add this in next pull request.
@ghost
Copy link
Author

ghost commented Feb 4, 2019

@maraf,

Sorry about all the random commits my brain is not in a right place right now. I have removed Staging and Production will add this in next pull request when I implement bootstrap layout.

@maraf maraf merged commit 5b0897a into maraf:master Feb 4, 2019
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