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

remove "What does it do?" settings listing: #229

Merged
merged 8 commits into from
Mar 13, 2017
Merged

remove "What does it do?" settings listing: #229

merged 8 commits into from
Mar 13, 2017

Conversation

nodiscc
Copy link
Contributor

@nodiscc nodiscc commented Mar 1, 2017

Outdated, see below.

@nodiscc
Copy link
Contributor Author

nodiscc commented Mar 1, 2017

The last commit d312f51 removes 2 preferences that I think are unneeded/incorreclty described/need more research.

@nodiscc
Copy link
Contributor Author

nodiscc commented Mar 1, 2017

Outdated, see below

Copy link
Owner

@pyllyukko pyllyukko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d312f51: I think these should be handled in separate issue/PR

@pyllyukko
Copy link
Owner

I'm not entirely sure about all this. I mean I get the point that it would be nice to be able to just update one place where setting specific information is, but I'm not sure whether it's so easily resolved.

I also realize, that the "highlights" part needs some rework/review/update.

Also the README and the explanations profit from markdown syntax so we can do formatting, links etc., which we can't put into the .js file.

@nodiscc
Copy link
Contributor Author

nodiscc commented Mar 6, 2017

d312f51 Remove gfx.direct2d.disabled layers.acceleration.disabled : I think these should be handled in separate issue/PR

Moved to #230, reverted here.

@nodiscc
Copy link
Contributor Author

nodiscc commented Mar 6, 2017

I get the point that it would be nice to be able to just update one place where setting specific information is
"highlights" part needs some rework/review/update.
README and the explanations profit from markdown syntax so we can do formatting, links etc

Yes that is the point of this PR. I figured the best course would be

  1. Deduplicating links and moving them alongside respective prefs (2c0c031)
  2. Automatically generating markdown from information in the user.js file (cc241d3, incomplete work, see below)

@pyllyukko Regarding 2. could you please provide an example of a final, markdown formatted section you consider appropriate? (For example what should the final HTML5/DOM APIs section look like).

which we can't put into the .js file.

Such info can be added to the section header/other comments/... would you agree? Or we can find a workaround. The point is, manual management of "what does it do?" is high-maintenance, error-prone, etc.

@nodiscc
Copy link
Contributor Author

nodiscc commented Mar 7, 2017

Makefile updated.
Current output can be seen at https://gist.github.com/nodiscc/43e75ab753efadb718302d91d9f37511
Let me know if you're ok with that method, and if it requires further changes.

If you agree, I will proceed with:

  • Adding a short description for each preference/prefs block.
  • Trimming section descriptions to a single line.

@pyllyukko
Copy link
Owner

@pyllyukko Regarding 2. could you please provide an example of a final, markdown formatted section you consider appropriate? (For example what should the final HTML5/DOM APIs section look like).

I think we should be able to generate (almost) similar than what it is now. For the most parts it could be possible, but for instance the longer paragraphs in the beginning of the sections could be static in the README. Maybe the related links could be after the description, e.g. * Disable Service Workers[1][2][3].

Such info can be added to the section header/other comments/... would you agree? Or we can find a workaround.

In the README that is?

The point is, manual management of "what does it do?" is high-maintenance, error-prone, etc.

That is absolutely true.

nodiscc added 2 commits March 9, 2017 04:08
…ts/organization

* Doc improvements/reorganization:
  * user.js: improve/simplify preferences descriptions, move multiline descriptions to single line
  * reorganize: move all test suites to README, sort/group tests, reorder links, remove duplicates

* Automate README 'What does it do' section generation:
  * README: remove hardcoded markdown "What does it do?" settings listing
    * move information to relevant user.js prefs/sections
  * user.js: add SECTION: prefix for section titles, add PREF: prefix for preferences
  * Add a basic makefile with a single target
  * Add a shell script 'gen-readme.sh' to generate 'What does it do' section:
    * For each section header in user.js, display a generaic description of the section
      Custom markdown text can be set for each section in 'gen-readme.sh' (config section)
      user.js: remove section descriptions, move to config options in gen-readme.sh
    * For each preference, append a list of reference links from user.js
    * Write the section to README.md, delimited by HTML tags BEGIN/END SECTION
* Misc. removals:
  * Remove tshark/wireshark snippet: tshark -t ad -n -i wlan0 -T text -V -R ssl.handshake
  * remove "to use developer tools in the context of the browser itself, and not only web content"

Part of #220
@nodiscc
Copy link
Contributor Author

nodiscc commented Mar 9, 2017

Rebased as 2 commits: https://github.com/pyllyukko/user.js/pull/229/commits

Preview: https://github.com/nodiscc/user.js/blob/wiki-migration/README.md#what-does-it-do

    Automate README 'What does it do' section generation, doc. improvements/organization
    
    * Doc improvements/reorganization:
      * user.js: improve/simplify preferences descriptions, move multiline descriptions to single line
      * reorganize: move all test suites to README, sort/group tests, reorder links, remove duplicates
    
    * Automate README 'What does it do' section generation:
      * README: remove hardcoded markdown "What does it do?" settings listing
        * move information to relevant user.js prefs/sections
      * user.js: add SECTION: prefix for section titles, add PREF: prefix for preferences
      * Add a basic makefile with a single target
      * Add a shell script 'gen-readme.sh' to generate 'What does it do' section:
        * For each section header in user.js, display a generaic description of the section
          Custom markdown text can be set for each section in 'gen-readme.sh' (config section)
          user.js: remove section descriptions, move to config options in gen-readme.sh
        * For each preference, append a list of reference links from user.js
        * Write the section to README.md, delimited by HTML tags BEGIN/END SECTION
    * Misc. removals:
      * Remove tshark/wireshark snippet: tshark -t ad -n -i wlan0 -T text -V -R ssl.handshake
      * remove "to use developer tools in the context of the browser itself, and not only web content"
    
    Part of #220

   * run 'make', update README 'what does it do' section from user.js

@nodiscc
Copy link
Contributor Author

nodiscc commented Mar 9, 2017

Added a commit: 686a4c0

issue 208 item 401 (extensions.blocklist), improve OCSP doc, enable OCSP must-staple

 * Add extensions.blocklist.url pref, Fixes item 0401 Sanitize blocklist url of #208
 * improve documentation on extensions.blocklist.enabled
 * add services.blocklist.update_enabled = true
 * improve documentation on OCSP
 * enable OCSP must-staple extension

nodiscc added 4 commits March 9, 2017 19:57
…CSP must-staple

 * Add extensions.blocklist.url pref, Fixes item 0401 Sanitize blocklist url of #208
 * improve documentation on extensions.blocklist.enabled
 * add services.blocklist.update_enabled = true
 * improve documentation on OCSP
 * enable OCSP must-staple extension
@pyllyukko
Copy link
Owner

Preview: https://github.com/nodiscc/user.js/blob/wiki-migration/README.md#what-does-it-do

I have to admit that it's starting to look good :)

@nodiscc
Copy link
Contributor Author

nodiscc commented Mar 12, 2017

@pyllyukko I think this could be merged in the current state. There are still prefs named ?? but filling in appropriate names can be done after this is merged. Just run make to auto-update the README.

Merging this PR would be good because it is becoming hard to rebase on/merge master every time master is updated.

Note that there's a pref addition in 686a4c0, and 2 readme link additions in 71fc34b and 88f0ae0. Other changes are just reorganization + adding the generation script (692ba68), and letting the script update README.md (b03059b, 4eadc12)

@pyllyukko
Copy link
Owner

Ok. So everything regarding this is now implemented? I'll need to review this more thoroughly before merging. I'll refrain from pushing any more commits before addressing this.

I'm afraid we'll need one more rebase still :)

@nodiscc
Copy link
Contributor Author

nodiscc commented Mar 12, 2017

I've merged master (easier than rebasing). I'll wait for your review before working on other tasks. Thanks :)

Just a reminder:

  • "What does it do?" subsections descriptions are now defined (using markdown) in gen-readme.sh
  • This section can now be automatically updated by running make
  • It requires that preference descriptions are prefixed with // PREF: to work as expected.

@pyllyukko
Copy link
Owner

Should we ditch the commented out settings from the README? I see that you have just included a "(disabled)" suffix, but maybe we don't need to clutter the README with those.

@nodiscc
Copy link
Contributor Author

nodiscc commented Mar 13, 2017

I can prevent generating README entries for (disabled) items.
Edit: done

@pyllyukko
Copy link
Owner

I'm trying to merge this today, as I'm quite busy with other stuff for the rest of this week. You can see my changes so far in the nodiscc-wiki-migration branch.

@pyllyukko
Copy link
Owner

There are still prefs named ?? but filling in appropriate names can be done after this is merged.

I replaced all the "??" with at least the pref's name (6d92170). We need to make a separate issue to fill in the blanks.

@pyllyukko pyllyukko merged commit 10093fd into pyllyukko:master Mar 13, 2017
@pyllyukko
Copy link
Owner

Boom!

@pyllyukko
Copy link
Owner

I also created #236

@pyllyukko
Copy link
Owner

Huge thanks for the effort @nodiscc! Even though I was a bit sceptic in the beginning, this turned out to be a huge improvement regarding our documentation. I think it also helps us in the future because we need to write the description/reasoning for each pref and thus improving the overall quality of this project.

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