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

Update Grunt, remove unnecessary items from Gruntfile, register task 'default' #58

Merged
merged 2 commits into from
Oct 10, 2019

Conversation

benlk
Copy link
Collaborator

@benlk benlk commented Oct 10, 2019

Changes

This pull request makes the following changes:

  • Updates grunt dependencies
  • Drops package.lock
  • Removes the version task, because we haven't been versioning this software for release
  • Drops grunt-contrib-pot, grunt-confirm, grunt-git, grunt-po2mo, grunt-pot, and grunt-version as they're unused in this project. If we end up making this theme translatable, we should use grunt-shell as a wrapper for the wp i18n tools.

Why

This should clear up the npm-related security alerts for this repo, as well as bring improved syntax handling in LESS files for CSS' calc().

Testing/Questions

Features that this PR affects:

  • CSS/LESS compilation in dev mode

Questions that need to be answered before merging:

  • Is this PR targeting the correct branch in this repository?

Steps to test this PR:

  1. the CSS loads, right?
  2. Run grunt and watch for errors: there should be none, and the default task should be run.

@benlk benlk requested a review from joshdarby October 10, 2019 19:11
@benlk benlk added this to In progress in CITY-008 - Website Redesign via automation Oct 10, 2019
@joshdarby
Copy link

@benlk This works fine, but something in either this or the PR that was just merged (#33) is causing the main nav dropdowns to not display.

@benlk
Copy link
Collaborator Author

benlk commented Oct 10, 2019

Yep, I'm seeing that with #33 checked out as well. Will put a patch in this PR.

@joshdarby
Copy link

@benlk Do you want help narrowing down what happened or do you have an idea?

@benlk
Copy link
Collaborator Author

benlk commented Oct 10, 2019

Assistance would be welcome.

@benlk
Copy link
Collaborator Author

benlk commented Oct 10, 2019

It's not WPBuddy/largo#1715

@benlk
Copy link
Collaborator Author

benlk commented Oct 10, 2019

@joshdarby does this go away for you if in Appearance > CSS Variables you hit "Save Variables"?

@joshdarby
Copy link

@benlk Sure does.

@benlk
Copy link
Collaborator Author

benlk commented Oct 10, 2019

Then it sounds like both of us had pre-Largo-0.6.4 CSS compiled, but since that isn't updated automatically when Largo updates (we require a human to push the button) we had to clear the old styles and generate new ones. It's not a problem with this PR or #33, or even this theme.

(And WPBuddy/largo#1137 would solve the problem by getting rid of that cached stylesheet.)

OK to merge?

@joshdarby
Copy link

Awesome. 👍

CITY-008 - Website Redesign automation moved this from In progress to Reviewer approved Oct 10, 2019
@benlk benlk merged commit 27e68a9 into master Oct 10, 2019
CITY-008 - Website Redesign automation moved this from Reviewer approved to Done Oct 10, 2019
@benlk benlk deleted the grunt-update branch October 10, 2019 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants