Skip to content
This repository has been archived by the owner on Jun 18, 2024. It is now read-only.

Remove LESS #303

Merged
merged 14 commits into from
Feb 5, 2016
Merged

Remove LESS #303

merged 14 commits into from
Feb 5, 2016

Conversation

mikewheaton
Copy link
Contributor

This should get us all the way to completing #56. @battletoilet, please have a look at the build process to see if I've missed anything.

@Jahnp
Copy link
Collaborator

Jahnp commented Feb 1, 2016

Have you checked out the copy/build steps in FabricBuild.js? Since you've removed the LESS dependencies but not the tasks that use them, those tasks will fail (hence the failing build).

@mikewheaton
Copy link
Contributor Author

Working on fixing the build now...

// Style Linting
// ----------------------------------------------------------------------------

gulp.task('ComponentSamples-styleHinting', function() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting--is there no linting for SASS built in that we can switch to? We definitely want that included, very soon if not right now in this PR. I realize that's scope creep but it's an important aspect of development we shouldn't lose.

@Jahnp
Copy link
Collaborator

Jahnp commented Feb 4, 2016

Overall, looks really good. Biggest issue I can see is a lack of SASS linting/error reporting now that LESS is no longer part of the build at all.

Also, this probably doesn't need to be addressed in this PR, we should consider whether we need some of the leftover config bits from supporting 2 preprocessors, e.g. running a task for setting SassMode explicitly (it's the only mode now) and the sassExtension prop in Config.js.

@Jahnp
Copy link
Collaborator

Jahnp commented Feb 4, 2016

Also, please remove component-manifest-template.less--that's no longer needed either.

John Miller added 3 commits February 4, 2016 14:45
…into miwhea/remove-less

# Conflicts:
#	src/components/Spinner/Spinner.less [Resolved]
#	src/less/Fabric.RTL.less [Resolved]
#	src/less/Fabric.less [Resolved]
gokunymbus added a commit that referenced this pull request Feb 5, 2016
@gokunymbus gokunymbus merged commit c722fe8 into master Feb 5, 2016
@mikewheaton mikewheaton mentioned this pull request Feb 5, 2016
@mikewheaton mikewheaton deleted the miwhea/remove-less branch February 9, 2016 03:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants