[v6] Add a banner on the old documentation docs

Message ID 20220421220614.30390-1-abongwabonalais@gmail.com
State New
Headers show
Series [v6] Add a banner on the old documentation docs | expand

Commit Message

Abongwa Bonalais April 21, 2022, 10:06 p.m. UTC
Signed-off-by: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com>
---
 htmlscript.py  | 36 +++++++++++++++++++++++++++++
 stylescript.py | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+)
 create mode 100644 htmlscript.py
 create mode 100644 stylescript.py

Comments

Quentin Schulz April 22, 2022, 8:30 a.m. UTC | #1
Hi Amahnui,

On April 22, 2022 12:06:14 AM GMT+02:00, Abongwa Amahnui Bonalais <abongwabonalais@gmail.com> wrote:
>Signed-off-by: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com>
>---
> htmlscript.py  | 36 +++++++++++++++++++++++++++++
> stylescript.py | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++

Please have those files inside scripts/ in yocto-autobuilder-helper git repo and send a patch following the contribution guidelines (yocto-autobuilder-helper in the tag part of the commit title ([PATCH yocto-autobuilder-helper]). You did it correctly for the other mail you just sent, the same needs to be done for this one too.

As a side note, the loops are identical in both scripts, so handling html and css files within the same loop with just a different if condition would have been better and avoided a need for a second file.

> 2 files changed, 97 insertions(+)
> create mode 100644 htmlscript.py
> create mode 100644 stylescript.py
>
>diff --git a/htmlscript.py b/htmlscript.py
>new file mode 100644
>index 0000000..fdbd109
>--- /dev/null
>+++ b/htmlscript.py
>@@ -0,0 +1,36 @@
>+#!/usr/bin/env python3
>+#
>+# SPDX-License-Identifier: GPL-2.0-only
>+#
>+#Signed-off-by: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com>
>+#
>+#
>+# function to append to the content of a html file below the body tag

It would help to explain why this is needed. (Display the obsolete banner on the old docs website)

>+#
>+#
>+import os
>+html_content = '''
>+<div id="outdated-warning">This document is outdated, you should select the <a href="https://docs.yoctoproject.org/">latest release version</a> in this series.</div>
>+<div xml:lang="en" class="body" lang="en">
>+'''
>+last_div = '''
>+</div>
>+
>+'''
>+
>+def loop_through_html_directories(dir):
>+    for dirpath, dirnames, filenames in os.walk(dir):
>+    # loop through all files in the directory
>+        for filename in filenames:
>+            # check if the file is an html file
>+            if filename.endswith('.html'):

i.e. have here another if filename.endswith('style.css'): and have the code of the other file in there.

>+                # open the html file in read mode
>+                with open(os.path.join(dirpath, filename), 'r', encoding="ISO-8859-1") as f:
>+                    # read the content of the html file
>+                    current_content = f.read()
>+                # open the html file in write mode
>+                with open(os.path.join(dirpath, filename), 'w') as f:
>+                    # write the content of the html file
>+                    f.write(current_content.replace('<body>', '<body>' + html_content))
>+                    f.write(current_content.replace('</body>', last_div + '</body>'))
>+loop_through_html_directories('.')
>diff --git a/stylescript.py b/stylescript.py
>new file mode 100644
>index 0000000..2777730
>--- /dev/null
>+++ b/stylescript.py
>@@ -0,0 +1,61 @@
>+#!usr/bin/env python3
>+#
>+# SPDX-License-Identifier: GPL-2.0-only
>+#
>+#Signed-off-by: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com>
>+#
>+#
>+# function to append to the content of a css file below the body class
>+#
>+#
>+import os
>+
>+
>+content = '''
>+ 
>+  font-family: Verdana, Sans, sans-serif;
>+
>+  /*min-width: 640px;*/
>+  width: 100%;
>+  margin:  0;
>+  padding: 0;
>+  color: #333;
>+  overflow-x: hidden;
>+  }
>+ 
>+ /*added books too*/
>+.body{
>+margin:  0 auto;
>+min-width: 640px;
>+padding: 0 5em 5em 5em;
>+}
>+/* added the id below to make the banner show and be fixed*/
>+#outdated-warning{
>+text-align: center;
>+background-color: rgb(255, 186, 186); 
>+color: rgb(106, 14, 14); 
>+padding: 0.5em 0; 
>+width: 100%;
>+position: fixed;
>+top: 0;
>+
>+
>+'''
>+
>+
>+
>+def loop_through_html_directories(dir):
>+    for dirpath, dirnames, filenames in os.walk(dir):
>+    # loop through all files in the directory
>+        for filename in filenames:
>+            # check if the file is an html file

Incorrect comment, we're looking for style.css.

>+            if filename.endswith('style.css'):
>+                # open the html file in read mode

Incorrect comments we're opening style.css

>+                with open(os.path.join(dirpath, filename), 'r', encoding="ISO-8859-1") as f:
>+                    # read the content of the html file

Same here.

>+                   css_content = f.read()
>+                # open the html file in write mode

Same here.

>+                with open(os.path.join(dirpath, filename), 'w') as f:
>+                    # write the content of the html file
>+                   

Same here.
 f.write(css_content.replace(css_content[css_content.find('body {'):css_content.find('}'[0])], 'body {' + content ))
>+loop_through_html_directories('.')

A better name would be fix_all_style_css('.')
Or something like that. Because, yes, you're looping through directories but that's not what this function is used for, it's used for fixing files.

Same for the function in the other file.

The script filename would benefit from a rename too, likely docs_fix_all_style_css.py or something similar. So that we know what this script is used for without reading it.

The filenames are just suggestions, if you come up with something better, please use whatever think is best.

Cheers,
Quentin
Abongwa Bonalais April 22, 2022, 1:17 p.m. UTC | #2
Hi Quentin,
Thank you very much for the review, I will apply all the changes based on your review and send another patch.
Abongwa Bonalais April 22, 2022, 1:53 p.m. UTC | #3
> 
> A better name would be fix_all_style_css('.') Or something like that.
> Because, yes, you're looping through directories but that's not what this
> function is used for, it's used for fixing files. Same for the function in
> the other file. The script filename would benefit from a rename too, likely
> docs_fix_all_style_css.py or something similar. So that we know what this
> script is used for without reading it. The filenames are just suggestions,
> if you come up with something better, please use whatever think is best.
> 

Since I am to send everything to a single script, I will just give it a generic name to cover both the html and css changes.
Abongwa Bonalais April 22, 2022, 2:51 p.m. UTC | #4
Hi Quentin,
I sent a more recent patch containing the changes you requested.

Patch

diff --git a/htmlscript.py b/htmlscript.py
new file mode 100644
index 0000000..fdbd109
--- /dev/null
+++ b/htmlscript.py
@@ -0,0 +1,36 @@ 
+#!/usr/bin/env python3
+#
+# SPDX-License-Identifier: GPL-2.0-only
+#
+#Signed-off-by: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com>
+#
+#
+# function to append to the content of a html file below the body tag
+#
+#
+import os
+html_content = '''
+<div id="outdated-warning">This document is outdated, you should select the <a href="https://docs.yoctoproject.org/">latest release version</a> in this series.</div>
+<div xml:lang="en" class="body" lang="en">
+'''
+last_div = '''
+</div>
+
+'''
+
+def loop_through_html_directories(dir):
+    for dirpath, dirnames, filenames in os.walk(dir):
+    # loop through all files in the directory
+        for filename in filenames:
+            # check if the file is an html file
+            if filename.endswith('.html'):
+                # open the html file in read mode
+                with open(os.path.join(dirpath, filename), 'r', encoding="ISO-8859-1") as f:
+                    # read the content of the html file
+                    current_content = f.read()
+                # open the html file in write mode
+                with open(os.path.join(dirpath, filename), 'w') as f:
+                    # write the content of the html file
+                    f.write(current_content.replace('<body>', '<body>' + html_content))
+                    f.write(current_content.replace('</body>', last_div + '</body>'))
+loop_through_html_directories('.')
diff --git a/stylescript.py b/stylescript.py
new file mode 100644
index 0000000..2777730
--- /dev/null
+++ b/stylescript.py
@@ -0,0 +1,61 @@ 
+#!usr/bin/env python3
+#
+# SPDX-License-Identifier: GPL-2.0-only
+#
+#Signed-off-by: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com>
+#
+#
+# function to append to the content of a css file below the body class
+#
+#
+import os
+
+
+content = '''
+ 
+  font-family: Verdana, Sans, sans-serif;
+
+  /*min-width: 640px;*/
+  width: 100%;
+  margin:  0;
+  padding: 0;
+  color: #333;
+  overflow-x: hidden;
+  }
+ 
+ /*added books too*/
+.body{
+margin:  0 auto;
+min-width: 640px;
+padding: 0 5em 5em 5em;
+}
+/* added the id below to make the banner show and be fixed*/
+#outdated-warning{
+text-align: center;
+background-color: rgb(255, 186, 186); 
+color: rgb(106, 14, 14); 
+padding: 0.5em 0; 
+width: 100%;
+position: fixed;
+top: 0;
+
+
+'''
+
+
+
+def loop_through_html_directories(dir):
+    for dirpath, dirnames, filenames in os.walk(dir):
+    # loop through all files in the directory
+        for filename in filenames:
+            # check if the file is an html file
+            if filename.endswith('style.css'):
+                # open the html file in read mode
+                with open(os.path.join(dirpath, filename), 'r', encoding="ISO-8859-1") as f:
+                    # read the content of the html file
+                   css_content = f.read()
+                # open the html file in write mode
+                with open(os.path.join(dirpath, filename), 'w') as f:
+                    # write the content of the html file
+                    f.write(css_content.replace(css_content[css_content.find('body {'):css_content.find('}'[0])], 'body {' + content ))
+loop_through_html_directories('.')