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

Duplicate CSS rules in bootstrap.css #11533

Closed
kkobashi opened this issue Nov 19, 2013 · 5 comments
Closed

Duplicate CSS rules in bootstrap.css #11533

kkobashi opened this issue Nov 19, 2013 · 5 comments

Comments

@kkobashi
Copy link

.container {
padding-right: 15px;
padding-left: 15px;
margin-right: auto;
margin-left: auto;
}

.container:before, #1
.container:after {
display: table;
content: " ";
}

.container:after { #2
clear: both;
}

.container:before, DUPLICATE OF #1
.container:after {
display: table;
content: " ";
}

.container:after { DUPLICATE OF #2
clear: both;
}

.row {
margin-right: -15px;
margin-left: -15px;
}

.row:before, #3
.row:after {
display: table;
content: " ";
}

.row:after { #4
clear: both;
}

.row:before, DUPLICATE OF #3
.row:after {
display: table;
content: " ";
}

.row:after { DUPLICATE OF #4
clear: both;
}

@cvrebert
Copy link
Collaborator

Duplicate of #8947. Please search next time.

@kkobashi
Copy link
Author

#8947's title is specifically "Duplicate entries in each clearfix"

Noone is going to look further into that bug to come to the conclusion that its a catch all for duplicate CSS issues!

Bug titles are supposed to be specific so they can be tracked and handled on a case by case basis.

The way #8947 reads is that it deteriorated into a catch all for all duplicate CSS issues. But the title doesn't do it any justice. Going through a discussion about a specific topic that masks itself as a more generic discussion is horrendous QA bug keeping.

The danger is someone is going to screwup and ignore each separate issue on its own merit.

@carasmo
Copy link

carasmo commented Nov 19, 2013

Yes, but what you posted is the .clearfix(); mixin duplicate bug found in all the cases you pasted. That's the only place, due to the lack of support of LESS 1.4 and up in the compiling app that Bootstrap uses (many of the compiling scripts are not upgraded to the latest less).

@kkobashi
Copy link
Author

That is understood now and only if you read #8947 in the first place. Most people didnt even get there. They had to be told to go there - a big difference.

Step back for a moment and look at how people report bugs. They first have to summarize the issue and then come up with keywords that are close to it. "Duplicate" comes up with 787 issues to read through. "Duplicate css" comes up with 221. You click on the relevant titles.

Does "duplicate entries in each clearfix" stick out to you? Is it intuitive enough to be the baseline catch all for all duplicate css issues in bootstrap.css? It doesn't to me and neither to the other people who used search and created all the duplicate bugs!

What I'm trying to say here is if there is a duplicate bug and you start referencing it from other bug records, you have to make sure the title and description fit the generality of the situation. If not, change the title and the description so it can be. Don't suggest that the person didn't search because it can very well be your poor decision to use a bug record as a baseline with poor title and long threaded discussion to arrive at this conclusion.

And if you start seeing too many bugs of the same type, you have to go the extra mile and communicate this in the documentation. Its too late to create a bug record that would serve as the baseline when you have used too many duplicate status along with bug references that need to be changed to point at it.

Over the years I worn hats at all triage levels (lead developer, lead qa, project manage). One thing I found frustrating is that my QA team would automatically (out of laziness) mark a bug as duplicate without investigating more into it. They naturally assumed it was of the same type.

Who's to say in this case that this isn't pilot error on the developer side and she accidentally cut,copy,paste code to duplicate the CSS, rather than this being a compiler issue? Marking bugs as being duplicated is an inclination. You should never close a duplicate bug until you have tested for it. As a QA engineer, never assume!

@mdo
Copy link
Member

mdo commented Nov 20, 2013

Point taken @kkobashi. We get rather brief with folks given the volume of issues we deal with, and as such our tone can be mistaken as hostile rather then a succinct and directive reply. It's something we've tried to figure out as we go.

That said, all the instances you mentioned are from the clearfix mixin. Similar to us doing more work to help folks search and identify issues, and as @carasmo is alluding to, you could do more by looking into where that generated CSS comes from to help you narrow your search. A quick search for those repeated styles within our code base would then lead you to the clearfix mixin, and hopefully then to #8947.

Agreed on the renaming titles of issues—that'd definitely help moving forward.

<3

mdo added a commit that referenced this issue Dec 9, 2013
Original discussion:
less/less.js#1437 (comment).

Since we’re switching to `grunt-contrib-less`, we can take advantage of
newer LESS features than what RECESS supported. Included in that is the
ability to `:extend`, and not only that, but `:extend(.mixin-name
all)`. By doing so, we remove duplicate CSS for all our elements that
were being clearfix-ed.

Fixes #8947, #8968, #8991, #9257, #9268, #9291, #9430, #9604, #9686,
#9929, #10731, #10793, #11305, #11498, #11533, #11570, #11604, #11652.

(dem issues, tho)
stempler pushed a commit to stempler/bootstrap that referenced this issue Apr 11, 2014
Original discussion:
less/less.js#1437 (comment).

Since we’re switching to `grunt-contrib-less`, we can take advantage of
newer LESS features than what RECESS supported. Included in that is the
ability to `:extend`, and not only that, but `:extend(.mixin-name
all)`. By doing so, we remove duplicate CSS for all our elements that
were being clearfix-ed.

Fixes twbs#8947, twbs#8968, twbs#8991, twbs#9257, twbs#9268, twbs#9291, twbs#9430, twbs#9604, twbs#9686,
twbs#9929, twbs#10731, twbs#10793, twbs#11305, twbs#11498, twbs#11533, twbs#11570, twbs#11604, twbs#11652.

(dem issues, tho)
stempler pushed a commit to stempler/bootstrap that referenced this issue Nov 4, 2014
Original discussion:
less/less.js#1437 (comment).

Since we’re switching to `grunt-contrib-less`, we can take advantage of
newer LESS features than what RECESS supported. Included in that is the
ability to `:extend`, and not only that, but `:extend(.mixin-name
all)`. By doing so, we remove duplicate CSS for all our elements that
were being clearfix-ed.

Fixes twbs#8947, twbs#8968, twbs#8991, twbs#9257, twbs#9268, twbs#9291, twbs#9430, twbs#9604, twbs#9686,
twbs#9929, twbs#10731, twbs#10793, twbs#11305, twbs#11498, twbs#11533, twbs#11570, twbs#11604, twbs#11652.

(dem issues, tho)
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

No branches or pull requests

4 participants