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

Office hours 2020 02 21 #41

Merged
merged 7 commits into from
May 29, 2020
Merged

Office hours 2020 02 21 #41

merged 7 commits into from
May 29, 2020

Conversation

benlk
Copy link
Collaborator

@benlk benlk commented Feb 21, 2020

WIP PR at stopping point where we noticed #40.

Changes

This pull request makes the following changes:

Why

For #34

Testing/Questions

Features that this PR affects:

Questions that need to be answered before merging:

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

Steps to test this PR:

@benlk benlk added this to the Homepage update quicksign milestone Feb 21, 2020
@benlk benlk self-assigned this Feb 21, 2020
@benlk
Copy link
Collaborator Author

benlk commented Feb 28, 2020

Deploying this to staging for review this afternoon.

@benlk
Copy link
Collaborator Author

benlk commented Feb 28, 2020

Deployed to http://voiceofoc.staging.wpengine.com/ at c1a55df.

@benlk
Copy link
Collaborator Author

benlk commented Mar 25, 2020

After this office hours, Sonya diffed the edited stylesheet on staging against the stylesheet from Git, and moved her changes into the Customizer's Additional CSS control.

At this point, if the edited stylesheet on staging is satisfactory, I'd be in favor of copying that over the new stylesheets in this Git repo, replacing the generated LESS files.

At that point, it becomes a question of whether we want to invest the time involved in backporting the CSS changes to LESS. That's a massive undertaking if there are extensive differences.

If Voice of OC plans to continue editing their CSS stylesheets by hand, I do not think we should plan to use LESS in this repo in the future, because we'd have to monitor the stylesheets for changes and backport them to the LESS. If they continue editing the CSS by hand, the CSS becomes the source of truth rather than the LESS, and there's no point maintaining the LESS anymore. Delete the LESS, delete Grunt, and remove the minified style.min.css from the staging site so that there's no confusion about whether the minified or the unminified file is canonical. Let there be only one CSS file.

@benlk
Copy link
Collaborator Author

benlk commented Mar 25, 2020

To make it easier to compare staging and prod and git in the future:

  • copy CSS files and functions.php from staging into this repo
  • delete whichever of style.css or style.min.css is not presently loaded on staging, and remove the reference to it from functions.php
  • delete Grunt and LESS

@benlk
Copy link
Collaborator Author

benlk commented Mar 27, 2020

Stylesheets on staging were not edited, and functions.php currently points to style.css. No changes are necessary at this time, so I'm not going to delete Grunt or LESS just yet.

I will include the fix for #36 and WPBuddy/largo#1855 from INN/umbrella-mstoday#73.

@benlk
Copy link
Collaborator Author

benlk commented May 29, 2020

This is currently deployed to staging and approved by Sonya.

@benlk benlk marked this pull request as ready for review May 29, 2020 18:39
@joshdarby joshdarby merged commit 17c44e7 into staging May 29, 2020
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