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

docs: Add autoprefixer. #3803

Closed
wants to merge 1 commit into from
Closed

docs: Add autoprefixer. #3803

wants to merge 1 commit into from

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR commented Mar 3, 2019

Fixes #3797.

The browsers I went with are the ones from Bootstrap v4 just so that we get an idea. We can adapt it as needed of course :)

@coveralls
Copy link

coveralls commented Mar 3, 2019

Coverage Status

Coverage increased (+0.03%) to 91.719% when pulling f72957d on XhmikosR:xmr-autoprefixer into f1662ac on mochajs:master.

@Munter Munter self-assigned this Mar 3, 2019
@Munter
Copy link
Member

Munter commented Mar 3, 2019

This PR highlighted what we consider a flaw in the --browsers switch for assetgraph-builder and we are working on getting an improvement through the build pipelines.

When the update hits it should be enough to have a .browserslistrc and have autoprefixer installed.

We should of course run autoprefixer. I simply forgot it didn't come out of the box when I set up the build system. I'll make the adjustments for the upcoming version of assetgraph-builder on this branch and bump the version when it lands

package-scripts.js Outdated Show resolved Hide resolved
.browserslistrc Outdated Show resolved Hide resolved
@Munter Munter requested a review from plroebuck March 4, 2019 21:14
docs/.browserslistrc Outdated Show resolved Hide resolved
docs/.browserslistrc Outdated Show resolved Hide resolved
@plroebuck
Copy link
Contributor

So with the addition of "autoprefixer", is this no longer needed?

Not following how AG-B automagically runs "autoprefixer" for us using "/docs/.browserlistrc",
as "autoprefixer" is not an AG-B dependency.

package.json Show resolved Hide resolved
@XhmikosR
Copy link
Contributor Author

XhmikosR commented Mar 5, 2019

.browserslistrc doesn't work with the tool you are using thus why I had to add it here.

Not gonna comment on the other stuff, sorry.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Mar 5, 2019

And please don't edit my PRs from now on or I'll have to block this. You are doing things wrong.

@Munter
Copy link
Member

Munter commented Mar 6, 2019

@XhmikosR I was helping you get this past review and merged. You just reverted the work I did to make that happen. As a maintainer of the docs I find this disrespectful of the time I spent here, especially with the "doing things wrong" comment.

I'm closing this PR and doing the work myself instead then

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Mar 6, 2019

I'm sorry, but you don't know a single thing about this. I refuse to further engage to any discussion with you.

In fact I'm gonna close any PRs I made and move one. Good luck to the project with such maintainers as you.

@Munter
Copy link
Member

Munter commented Mar 6, 2019

As a maintainer of both the mocha docs, and a maintainer of the build system we use I respectfully disagree

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.

None yet

4 participants