-
Notifications
You must be signed in to change notification settings - Fork 464
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
Update VS build facade #2288
Update VS build facade #2288
Conversation
VALUE "OriginalFilename", "libsass.dll" | ||
VALUE "ProductName", "Libsass Library" | ||
VALUE "ProductVersion", "0.9.0.0" | ||
VALUE "ProductName", "LibSass Library" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has bugged me since I discovered this file :)
@am11 you're my hero 🎉 Are there any good resource on the recommended versioning schema for Windows, or is it fine to just mirror our libtool versioning? |
@xzyfer, I just mirrored the version numbers. As far as I can tell, the version conventions are not set in stone and there is quite a lot of variety of versions in win32 libs. For instance, instead of four-sequence identifier, we can even use three sequences (semver style). The RC file can read from other .h defines. I was thinking, if libtools and resource.rc share common .h file, that would make our life slightly easier. :) Alternatively, we can use a bumpversion.sh or extend git command, something like: |
@@ -18,14 +18,14 @@ BEGIN | |||
BEGIN | |||
BLOCK "080904b0" | |||
BEGIN | |||
VALUE "CompanyName", "Libsass Organization" | |||
VALUE "CompanyName", "LibSass Organization" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Sass Open Source Foundation" from the license?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will fix. Thanks.
MinimumVisualStudioVersion = 10.0.40219.1 | ||
Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "libsass", "libsass.vcxproj", "{E4030474-AFC9-4CC6-BEB6-D846F631502B}" | ||
EndProject | ||
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".SolutionItems", ".SolutionItems", "{33318C77-2391-4399-8118-C109155A4A75}" | ||
ProjectSection(SolutionItems) = preProject | ||
..\.editorconfig = ..\.editorconfig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, does this actually pick up the setting when you include it here? I was having problems when editing in VS the other day because it wasn't respecting it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to be working. I installed editorconfig for VS extension, opened libsass/win/libsass.sln
, set indent_size
to 12, opened libsass/src/file.cpp
and Ctrl+K,D to format the document, the intended 12 space indentation applied.
Note starting with VS2017, editorconfig and cmake have built-in support [ref].
@@ -1,10 +1,21 @@ | |||
| |||
Microsoft Visual Studio Solution File, Format Version 12.00 | |||
# Visual Studio 2013 | |||
VisualStudioVersion = 12.0.30723.0 | |||
# Visual Studio 14 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we dropping 2013 as the base version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think 2015 sln file works with 2013 and 2012, but not the lower ones. Besides, from command line perspective, msbuild
adapts to the installed toolchain.
Now that things are moving fast with the advent of VS Code for windOS and Unices, the previewed full VS for Mac and previewed VS2017, I honestly see no point in sticking with 2013 when we still can provide better support from XP-10 and all related server editions with latest CRT/CPP toolchains (brining dozens of bugfixes in STL and standard conformance). If there is a bug refraining us to moving forward, like any other piece of software, lets report the issue upstream, track it in our repo and keep the dice rolling forward when we are no backward compatibility trade-offs. Officially VC team hasn't even dropped XP support yet (released: 24.04.2001).
My €0.2 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure you're correct, since it's only the project toolsversion stuff that breaks backwards compat. Maybe AppVeyor should be updated too (separately)
Line 1 in 2847edc
os: Visual Studio 2013 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to issues with Node Sass we need to keep CI at VS2013.
VALUE "InternalName", "libsass" | ||
VALUE "LegalCopyright", "©2014 libsass.org" | ||
VALUE "LegalCopyright", "\251 2017 libsass.org" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libsass.org just redirects to sass-lang.com now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It redirects to the LibSass landing page. It's worth keeping for now.
ed018ed
to
2689261
Compare
Thanks for looking into this @am11. I don't think we need to complicate this any further. This is the first time we've bumped the C API ABI, so the process is new to me, and undocumented. |
Rebased the branch against master. |
Thanks. I'm in bed now, will take care of this in the morning.
…On 29 Jan. 2017 11:53 pm, "Adeel Mujahid" ***@***.***> wrote:
Rebased the branch against master.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2288 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAjZWA1sire1lPTmyG_awEvfWNdgJKrNks5rXIurgaJpZM4LcU4g>
.
|
@xzyfer, @mgreter,
I am not well familiar with the syntax of resource files. Just hacked it using Visual Studio's RC editor and updated only few parts (note that I manually updated the .rc file using sublime as editing + saving with VS results in multiple files noise such as .db, .aps and whatnot..)
Added a
.SolutionItems
folder to VS solution with some repo files including the resource.rc, so it gets visible in Solution Explorer of VS.Added .db and .aps to .gitignore.