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

Refactoring of the grunt build tool #891

Merged
merged 73 commits into from
Feb 8, 2016
Merged

Refactoring of the grunt build tool #891

merged 73 commits into from
Feb 8, 2016

Conversation

taylortom
Copy link
Member

What's new!

Instructions for testing!

  1. Pull code from here!
  2. Delete node_modules folder!
  3. Run npm install && grunt build && grunt server!

    If testing with the authoring tool, be aware that updating the git repo in /temp will cause certain deleted files/folders to be reinstated, so these will need to be manually removed (I think the main folder affected is src/course).

taylortom and others added 30 commits October 2, 2015 11:56
Had to separate all tasks into separate files
Help is currently broken, as jit-grunt is async
- The config file should be read in from the path passed in as an argument
- Logging enabled on 'server-build' task
- Commented out the verbose logging for excluded plugins for the time being
if (options.plugins) {
var pluginsClientSidePatch = 'requirejs.config({map: { "*": { "extensions/extensions":"'+options.pluginsModule+'","menu/menu":"'+options.pluginsModule+'","theme/theme":"'+options.pluginsModule+'","components/components":"'+options.pluginsModule+'" } } });';

if (!fs.existsSync(options.pluginsPath)) {
Copy link
Member

Choose a reason for hiding this comment

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

I think existsSync() has been deprecated. Can you consider using statSync() here instead?

@oliverfoster
Copy link
Member

i'm happy with the way this is working, so +1 for tom's bits

@taylortom
Copy link
Member Author

@oliverfoster was just about to do that, thanks 😁

@oliverfoster
Copy link
Member

✋ 🌴 🍻

@oliverfoster
Copy link
Member

can probably remove the console.log from https://github.com/adaptlearning/adapt_framework/blob/ABU-680/grunt/helpers.js#L51 now

@oliverfoster
Copy link
Member

+1 var testedOn = {"linux & mac" : "terminal", "windows10": [ "git-bash", "command line" ] };

@lc-thomasberger
Copy link
Member

+1 (tested on windows)

@dancgray
Copy link
Member

dancgray commented Feb 2, 2016

+1 (tested in Authoring tool on mac)

@moloko
Copy link
Contributor

moloko commented Feb 2, 2016

I'm assuming JSHint and JSCS still aren't fully integrated? I'm not seeing any alerts from them, is that as expected? If so then +1 (tested on Win 7 using Node v0.10.35 and cmder)

@brian-learningpool
Copy link
Member

+1 -- tested on develop branch of Authoring Tool on OS X. Great work @taylortom and @oliverfoster!

@taylortom
Copy link
Member Author

@moloko correct, you need to specifically run the commands at the moment. I've created a separate ticket to integrate these into travis.

If anyone else has some ideas about how we go about integrating this (if at all), will update this PR. @brian-learningpool @oliverfoster @dancgray @lc-thomasberger

...otherwise, let's get this baby merged! 💃💃

@oliverfoster
Copy link
Member

go for it

moloko added a commit that referenced this pull request Feb 8, 2016
Refactoring of the grunt build tool
@moloko moloko merged commit 53e12ef into master Feb 8, 2016
@moloko moloko deleted the ABU-680 branch February 8, 2016 11:45
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.

8 participants