Page MenuHomePhabricator

Add comments to the font-stacks to summarize the research since 2014's Typography Refresh
Closed, ResolvedPublic

Description

In addition to the parent task, I have written comments for the font stacks in the wikimedia-ui-base.less file for quick reference and explorability.

Patch: wikimedia-ui-base.less

Related tasks

font-family-* variable name cleanup: T245136: Font family variables: use consistent names
Source of the documentation: T244790: Document WikimediaUI Base's font stacks, the research and reasons for each choice
Touched upon: T247166: Deprecation of `@font-family-sans` in WikimediaUI Base (tracking)

Event Timeline

Demian renamed this task from Separate the font-stacks to a separate .less file and add a summary of the font documentation to Separate the font-stacks to a separate .less file and add comments to summarize the font documentation.Feb 10 2020, 9:54 PM
Demian created this task.
Demian renamed this task from Separate the font-stacks to a separate .less file and add comments to summarize the font documentation to Add comments to the font-stacks to summarize the font documentation, refactor to a separate .less file for clarity and SOC.Feb 11 2020, 2:05 AM
Demian updated the task description. (Show Details)
Demian renamed this task from Add comments to the font-stacks to summarize the font documentation, refactor to a separate .less file for clarity and SOC to Add comments to the font-stacks to summarize the font documentation, refactor to a separate .less file for clarity and separation of concerns.Feb 18 2020, 4:43 PM
Demian added a project: Documentation.

Change 573028 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/core@master] Intermediary solution for @import 'theme/wikimedia-ui-base';: duplicate/move the file to 'core/resources/src/mediawiki.less/theme' until a solution to import the original is implemented.

https://gerrit.wikimedia.org/r/573028

Change 573046 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[wikimedia-ui-base@master] Consistently follow the CSS variable naming convention with font variable names. Document the fonts.

https://gerrit.wikimedia.org/r/573046

Change 573062 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/core@master] Consistently follow the CSS variable naming convention with font variable names. Document the fonts.

https://gerrit.wikimedia.org/r/573062

Change 573433 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[wikimedia-ui-base@master] Update WMUI Base metadata, History.md for v0.15.0

https://gerrit.wikimedia.org/r/573433

Change 573435 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/core@master] Consistently follow the CSS variable naming convention with font variable names. Document the fonts.

https://gerrit.wikimedia.org/r/573435

@Demian We're tending not to provide such extensive code comments like proposed in the patch above to provide simple readability and skimmability in code in Foundation codebases. Documentation is too verbose
The patch is not following basic principles found throughout our codebase and commits. I don't see the capacity on my side to further review those if there are too many new conventions and ideas brought up, quickly proposed and changed in too short sequence.
Example: Additional description of the task reference in the commit message. Don't:

Bug: T245136  variable naming convention
Bug: T244794  Add comments

As general feedback:
The internet lives and breaths upon hyperlinks and references, we don't have to explain or point to every little source for decisions in code comments. Hyperlinks and git log/git blame provides devs sufficient insights.

I don't see this as useful or reasonable addition in the currently proposed way.

@Demian We're tending not to provide such extensive code comments

I've read your previous comment, you don't need to repeat in bold. I'm not following your practice of not documenting the code.

To address your request, I've removed the unnecessary comments and links. The rest of the comments is very important and the task's purpose is to add them. If these decisions were documented in the first place you would have saved me two weeks of research and for you the time to read the different cards I wrote in the process.

Obviously you are aware of the information that I've documented, however, any other person unfamiliar with the 10+ year history of these decisions are not. This experience in the last weeks shows these comments were missing. Please keep in mind the WMF is trying to attract contributors and support volunteers getting involved. Documentation is fundamental for this.

It's acceptable if you don't provide documentation for your products, but if someone makes the effort and offers to make that documentation then try to appreciate it and work with a cooperative mindset.
To do so: if you find the documentation extensive than please make a commit and remove the lines that you find unnecessary. That will take you less than an hour compared to the many days of research I've put into this task.

The patch is not following basic principles found throughout our codebase and commits.

Please clarify with examples, what basic principles you refer to. I've reiterated the patch multiple times based on your feedback. If there's something I've missed, please point to it with more clarity.

Example: Additional description of the task reference in the commit message. Don't:

Again, understand that this was helpful in development. It will be removed for the final patch, if you don't accept it.

As general feedback:
The internet lives and breaths upon hyperlinks and references, we don't have to explain or point to every little source for decisions in code comments. Hyperlinks and git log/git blame provides devs sufficient insights.

Finding information in git blame that was removed, or - God forbid - the file was renamed is difficult and time-consuming. Again, you obviously remember where to look, but this is not the case for anybody outside the dev team.

I don't see this as useful or reasonable addition in the currently proposed way.

Note that we are in disagreement. I wouldn't have spent a week researching and documenting this if it wasn't very helpful and it wouldn't save some other person a week's effort in the future.
If you wish to alter the commit and propose a shorter version, feel free to do so. Please be mindful of the effort that went into it and of the value of proper documentation.

Change 573609 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[wikimedia-ui-base@master] Add comments to the font-stacks to summarize the font documentation

https://gerrit.wikimedia.org/r/573609

@Demian We're tending not to provide such extensive code comments

I've read your previous comment, you don't need to repeat in bold. I'm not following your practice of not documenting the code.

You seem to be confused.

This is not a "I like apples, you like oranges" discussion of individual preferences with no impacts on other people.

This is Volker, as maintainer and lead in this area, informing you that it is our development practice and telling you not to do this, and that we document things in different places (specifically, the style guide that explains what we do and why we have come to those conclusions over years).

Hope this clarifies!

You seem to be confused.
Hope this clarifies!

No, no confusion on my part.
You might be aware that the style guide you're referring to does not document the decisions and the reasons, nor the fonts that were removed in the past and the reason for that.
This resulted in 2 weeks of my time and a few hours of yours wasted on finding the bits of discussions to put the pieces together. Please understand the value of that time and the value of documentation that will save that in the future.

I understand your practice is to not document and that's fine. However, if someone volunteers their time to fill in the gaps, then please endorse the wiki spirit of cooperation and value that someone does the hard task.

You seem to be confused.
Hope this clarifies!

No, no confusion on my part.
You might be aware that the style guide you're referring to does not document the decisions and the reasons, nor the fonts that were removed in the past and the reason for that.

Patches welcome. I think that would be welcome.

This resulted in 2 weeks of my time and a few hours of yours wasted on finding the bits of discussions to put the pieces together. Please understand the value of that time and the value of documentation that will save that in the future.

It's a lot more useful, and lot less work, if you start by talking to the experts first.

No one is disputing that this should be documented. I appreciate that you've spent an awful lot of time trying to reconstruct documentation retrospectively, and it would have been much better if we had maintained it such that you didn't need to do this.

I understand your practice is to not document and that's fine.

That's not remotely true. (And it wouldn't be fine.) Please see the Architecture Principles, the draft Architecture Guidelines, and other development policy documents.

However, if someone volunteers their time to fill in the gaps, then please endorse the wiki spirit of cooperation and value that someone does the hard task.

This isn't a wiki, and you shouldn't assume that it is. Areas are owned. Eventualism isn't sufficient. Civility is actually required. Many participants are paid for thier time. IAR doesn't apply. Etc.

It's a lot more useful, and lot less work, if you start by talking to the experts first.

This task has been here for 10 days, since 10 Feb, asking specific questions, still no answers. You've been pinged and didn't show up. There are 10+ outstanding questions in other topics I've addressed to you or your team in the last weeks. I hope you understand why I ignore this comment from you.

I understand your practice is to not document and that's fine.

That's not remotely true. (And it wouldn't be fine.) Please see the Architecture Principles, the draft Architecture Guidelines, and other development policy documents.

I'm not talking about the policies, but the actual practice. I've read some very well written documentation, I've worked on code that was sufficiently commented and generally I appreciate the lot of work that goes into documentation, translation, etc. Please understand my opinion that in this area the documentation should be more thorough. I'm talking from personal experience, we both have experienced it.

This isn't a wiki, and you shouldn't assume that it is. Areas are owned. Eventualism isn't sufficient. Civility is actually required. Many participants are paid for thier time. IAR doesn't apply. Etc.

Obviously I don't treat a source code repo as a wiki, I think you've misinterpreted me. I'm talking about the movement's fundamental value of cooperation that allows us, the encyclopedia, the software that runs it, to benefit from the work of everybody.

No one is disputing that this should be documented. I appreciate that you've spent an awful lot of time trying to reconstruct documentation retrospectively, and it would have been much better if we had maintained it such that you didn't need to do this.

If we can agree on that this documentation should be available, please let this discussion move on to what form it should take, specifically: where to commit the README-fonts.md, so that it's easily accessible at the moment a developer asks the question: "Why these fonts and why not that."

Demian triaged this task as Medium priority.Feb 28 2020, 6:52 PM
Demian updated the task description. (Show Details)

It's a lot more useful, and lot less work, if you start by talking to the experts first.

This task has been here for 10 days, since 10 Feb, asking specific questions, still no answers. You've been pinged and didn't show up.

Pinged for what? Also, 10 days for this kind of stuff is insanely fast; normally "hey, we should burn a bunch of time going back and researching work from years ago to document it better" gets answered in a few years, not a few days.

There are 10+ outstanding questions in other topics I've addressed to you or your team in the last weeks.

Which team do you think I'm in? I work with release engineering on site stability and maintenance, developer productivity, continuous integration, and code quality, and also assist colleagues acros the movement in getting complex changes done. I'm not in the Design team, so can't speak to your questions about specific font choices and how they came to the conclusions they did.

I agree with you that it is important to document the how the current stack represents hundreds of hours of research and testing across a number of people, and how the decisions made arose from those findings.

What questions are you looking for me, specifically, to answer?

I hope you understand why I ignore this comment from you.

You're of course welcome to "ignore" who you like, but you're very likely to get banned from gerrit and Phabricator with that attitude.

@Volker_E I reckon you haven't answered my questions in T244794#5900840 regarding your comments for more than a week and all your requests have been addressed, therefore I assume you agree with the latest patch. If you would still have any wishes regarding the latest patch set, please express it here.

Demian renamed this task from Add comments to the font-stacks to summarize the font documentation, refactor to a separate .less file for clarity and separation of concerns to Add comments to the font-stacks to summarize the research from 2014's Typography Refresh.Mar 4 2020, 2:43 AM
Demian updated the task description. (Show Details)
Demian renamed this task from Add comments to the font-stacks to summarize the research from 2014's Typography Refresh to Add comments to the font-stacks to summarize the research since 2014's Typography Refresh.Mar 4 2020, 3:36 AM

Change 577779 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/core@master] Update 'wikimedia-ui-base.less'

https://gerrit.wikimedia.org/r/577779

Change 577781 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/core@master] Add comments to the WMUI font stacks

https://gerrit.wikimedia.org/r/577781

Change 573065 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/MinervaNeue@master] Add comments to the WMUI font stacks

https://gerrit.wikimedia.org/r/573065

Change 577744 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] Add comments to the font-stacks, reference documentation

https://gerrit.wikimedia.org/r/577744

@Volker_E A friendly reminder that there was no response from you regarding the new patches addressing your requests in 3 weeks' time, while we were discussing other, unrelated matters. Yet, according to JdlRobson's comments and abruptly interrupted work together, it seems these patches are stalled on you. Do you have any comments about this and how to move forward?

Change 577744 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Add comments to the font-stacks, reference documentation

https://gerrit.wikimedia.org/r/577744

Change 573065 abandoned by Aron Manning:
Add comments to the WMUI font stacks

https://gerrit.wikimedia.org/r/573065

Change 573609 abandoned by VolkerE:
[wikimedia-ui-base@master] Add comments to the WMUI font stacks

Reason:
All useful comments requested have been added to font documentation either in WikimediaUI Base or in Design Style Guide. Latest was https://github.com/wikimedia/WikimediaUI-Style-Guide/pull/396. Hence abandoning.

https://gerrit.wikimedia.org/r/573609

Change 577779 abandoned by VolkerE:
[mediawiki/core@master] Update 'wikimedia-ui-base.less'

Reason:
Update is in the wrong repo. Already accomplished in OOUI in the meantime.

https://gerrit.wikimedia.org/r/577779

Change 577781 abandoned by VolkerE:
[mediawiki/core@master] Add comments to the WMUI font stacks

Reason:
Update is in the wrong repo. Already accomplished in OOUI in the meantime.

https://gerrit.wikimedia.org/r/577781

With latest links added to Design Style Guide, providing one source of truth to de-duplicate efforts and keep maintenance low, this seems resolved.