Fix: #545 making color of dropdown-item consistent

[krAtOsnana]

#545 make the colour of the dropDown-item consistent in all sizes of screen,
in file css/custom.css added a class for overriding the colour of dropdown-item .

#430 correcting the issue of bookmark in READM.md file ("Answers to a few questions" ).

[therealharshit]

Please add a description to your PR.

[krAtOsnana]

Please add a description to your PR.

Done.

[krAtOsnana]

Please add a description to your PR.

Is there anything else I should include in the description?

[quozl]

https://github.com/sugarlabs/sugar-docs/blob/master/src/contributing.md#making-commits explains commit message requirements. Looking through your commit messages first lines as shown by this GitHub pull request;

• there are more commits than expected, as if you are piling them on rather than collapsing, squashing or rebasing as you go,
• the first lines don't explain briefly what is done,
• you reference an issue number; sure, reference this in the pull request introduction comment so the GitHub automation works out, but don't put them in the commit message, as they won't have any meaning in future use of the git history.

[quozl]

Reviewed 892b132 in particular.

Please don't change Bootstrap source files unless you also convince the Bootstrap project to accept the change. You can see the soruce file by checking out tag v4.0.0 from the Bootstrap project at https://github.com/twbs/bootstrap ... when I do that, css/bootstrap.css in our project is identical to v4.0.0 dist/css/bootstrap.css in Bootstrap.

If you don't think your change is relevant to Bootstrap, but is specific to our project, please move your change into a file other than bootstrap.css.

[krAtOsnana]

Reviewed 892b132 in particular.

Please don't change Bootstrap source files unless you also convince the Bootstrap project to accept the change. You can see the soruce file by checking out tag v4.0.0 from the Bootstrap project at https://github.com/twbs/bootstrap ... when I do that, css/bootstrap.css in our project is identical to v4.0.0 dist/css/bootstrap.css in Bootstrap.

If you don't think your change is relevant to Bootstrap, but is specific to our project, please move your change into a file other than bootstrap.css.

i make changes in css/bootstrap.css file because change in 1 line of code ( color: #212529;) make the dropdown-itme's colour consistent

[krAtOsnana]

And i solve 2 issues i.e. #552 and #430 in a single branch thats why

https://github.com/sugarlabs/sugar-docs/blob/master/src/contributing.md#making-commits explains commit message requirements. Looking through your commit messages first lines as shown by this GitHub pull request;

• there are more commits than expected, as if you are piling them on rather than collapsing, squashing or rebasing as you go,
• the first lines don't explain briefly what is done,
• you reference an issue number; sure, reference this in the pull request introduction comment so the GitHub automation works out, but don't put them in the commit message, as they won't have any meaning in future use of the git history.

i solve 2 issues i.e. #552 and #430 in a single branch thats why it shows more commits.

[krAtOsnana]

how should i resolve this problem

[quozl]

i make changes in css/bootstrap.css file because change in 1 line of code ( color: #212529;) make the dropdown-itme's colour consistent

Yes, I know, but by doing it that way you create maintenance burden for future. In particular, each time we upgrade Bootstrap within this repository we would have to merge your change again and again. That's what we are resistant to. We've had that happen too much.

And i solve 2 issues i.e. #552 and #430 in a single branch thats why

Yes, I know, but the way in which you arranged the commits is strange; here's a screenshot of the pull request commits;

You've arranged each of these commits. You're asking us to consider each of them and in total.

how should i resolve this problem

Thanks for asking. I don't know what you did so I can't be specific in how to resolve the problem. You know what you did to reach this point. Consider what you did and what the effects were, and see if you can change what you did to yield different effects.

In general though, you should rewrite your branch with commits that

• make your changes to a file other than bootstrap.css,
• minimise the number of commits,
• explain in the commit messages what is being done and why, using our guidance on commit messages.

Then you can force push your branch and the pull request will be updated.

[quozl]

You may also be interested in how review happens, and what a reviewer is looking for. See https://github.com/sugarlabs/sugar-docs/blob/master/src/contributing.md#guide-for-reviewers

embedded.fm podcast also did an episode on reviews recently.

[krAtOsnana]

i make changes in css/bootstrap.css file because change in 1 line of code ( color: #212529;) make the dropdown-itme's colour consistent

Yes, I know, but by doing it that way you create maintenance burden for future. In particular, each time we upgrade Bootstrap within this repository we would have to merge your change again and again. That's what we are resistant to. We've had that happen too much.

And i solve 2 issues i.e. #552 and #430 in a single branch thats why

Yes, I know, but the way in which you arranged the commits is strange; here's a screenshot of the pull request commits;

You've arranged each of these commits. You're asking us to consider each of them and in total.

how should i resolve this problem

Thanks for asking. I don't know what you did so I can't be specific in how to resolve the problem. You know what you did to reach this point. Consider what you did and what the effects were, and see if you can change what you did to yield different effects.

In general though, you should rewrite your branch with commits that

• make your changes to a file other than bootstrap.css,
• minimise the number of commits,
• explain in the commit messages what is being done and why, using our guidance on commit messages.

Then you can force push your branch and the pull request will be updated.

reverted the changes done in bootstrap.css file #552 (comment) .
and making the desired dropdown-item color change in custom.css file with proper comments

[krAtOsnana]

@quozl please verify ?

[quozl]

Thanks.

• your changes for Question about "Answers to a few questions" referenced in README #430 do not satisfy the consensus in the issue,
• your changes for text color of "about us" option and "news" option #545 are sledgehammer approach; bootstrap.css specifies .dropdown-item colour of #212529 already, so could you please explain why that isn't working and you have to solve it this way? Explanation must be in the commit message.

Other less important things;

• you introduce a missing end of line on custom.css
• your commits have not been rewritten or squashed, again you've just added change upon change,
• commit description wording on fd3372b does not follow our guidance.

[pikurasa]

@krAtOsnana I recommend redoing this as two new PRs, each with clean commits as is suggested in this thread.

[krAtOsnana]

@krAtOsnana I recommend redoing this as two new PRs, each with clean commits as is suggested in this thread.
Ok sir , i am going to close this pull request because of certain reasons:
-> too much unclarified commits ,
-> not satisfying the issue.
-> solving more than one issue in a single thread.