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

docs: build 11ty site for tutorial and statement of work #19

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

bennypowers
Copy link

@bennypowers bennypowers commented Sep 22, 2022

What I did

This PR transforms both the statements-of-work (markdown, this repo) and the introductory tutorial (google doc, corporate google drive) into a website that uses red hat design system to provide a consistent and pleasant experience

Goals

  • Make the tutorial more accessible to students and more easily updated (via PRs)

Non-goals

  • content completeness - e.g. the homepage does not need to be fully developed in this PR

Comment on lines +18 to +32
links:
- heading: המוצרים שלנו
links:
- href: https://www.redhat.com/en/technologies/management/ansible
content: Red Hat Ansible Automation Platform
- href: https://www.redhat.com/en/technologies/linux-platforms/enterprise-linux
content: Red Hat Enterprise Linux
- href: https://www.redhat.com/en/technologies/cloud-computing/openshift
content: Red Hat OpenShift
- href: https://www.redhat.com/en/technologies/cloud-computing/openshift-data-foundation
content: Red Hat OpenShift Data Foundation
- href: https://www.redhat.com/en/technologies/linux-platforms/openstack-platform
content: Red Hat OpenStack Platform
- href: https://www.redhat.com/en/technologies/all-products
content: See all products

Choose a reason for hiding this comment

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

Do we need this for beyond?

Copy link
Author

Choose a reason for hiding this comment

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

we can either:

  1. replace the footer content/links with something more appropriate
  2. remove the main footer and use only the global (i.e. legal) footer

Copy link
Author

Choose a reason for hiding this comment

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

can we leave this for a subsequent PR?

Yarboa
Yarboa previously requested changes Sep 29, 2022
Copy link
Contributor

@Yarboa Yarboa left a comment

Choose a reason for hiding this comment

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

@bennypowers

There is no option for me, as "tech dumb" to track the amount of changes.
I think that @ifireball offline suggestions are correct:

... 
I would split that work into at least 4-5 separate PRs:
1. Suggest changes to the doc
2. Add 11ty in "Hello World" state - so only generated files
3. Add/move SOW files 
4. Add/move challenges doc files
...

My extra 2 cents here:

  • This is how a contributor promotes his PR/MR in OS communities for fast approvals.
  • In Agile methodologies, we are breaking down the pieces to achieve constant delivery.
    (PR is not moving forward quite long)
  • In addition as Beyond mentors we should lead by example.

I hope

@EdDev
Copy link

EdDev commented Sep 29, 2022

I would also recommend to keep the content independent of the presentation layer as much as possible.
It should be available as MD for the ones who maintain the content and as a backup in case the frontend will have maintenance issues.

@bennypowers
Copy link
Author

bennypowers commented Sep 29, 2022

@EdDev @Yarboa @ifireball

Atomic commits

split that work

I certainly sympathize with the need to review atomic changes in this PR. It happens to be that the commits in this PR are already atomic, so you might consider reviewing them one-by-one, which will allow you to review the changes in smaller chunks.
image

ex post facto, it would certainly have been better to take that approach, but as it is, picking apart this PR would take about the same amount of time as the original work - there is not a lot of code here - so since the changes here are small in the grand scheme of things, and the time it would take to split the existing atomic commits into separate PRs could be better spent on other things, and since this is a volunteer project, if there is room to be a little flexible in reviewing this PR, I would very much appreciate it. I'm also available to walk through the changes one-on-one if desired.


Content and Presention

content independent of the presentation layer

This is already the case in this PR. The content is markdown with frontmatter, the style and layout is all in the HTML templates, CSS, and eleventy config, according to Red Hat brand standards


Maintenance

tl;dr: there are basically 0 lines of build JS, and less than 200 lines of HTML for beyond to maintain here.

Aside from the content, the PR contains:

  • 3 HTML (nunjucks) templates, totaling 138 lines
  • 26 lines of eleventy configuration, most of that is just loading upstream plugins
  • 1 eleventy plugin (127 lines) which will shortly be upstreamed so that beyond does not have to maintain it
  • 9 direct npm dependencies, of which:
    • 3 are eleventy
    • 3 are red hat design system projects
    • 1 will be upstreamed away into the RHDS eleventy plugin
    • and the other two are eslint and yaml

That's not a tremendous amount of software to maintain, in my opinion. As well, the need for maintenance here is minimal, as software only runs in CI/CD, and there is very little security attack surface.

@bennypowers
Copy link
Author

As an aid to review, since deploy previews will only function if this PR is merged for subsequent PRs, I've attached to this message an archive of the site as built just now. unzip it and run a local webserver of your choice to review the site, or follow the instructions in README.md to build it yourself
beyond.zip

@bennypowers
Copy link
Author

@Yarboa WRT splitting the PR, we have another reason not to retroactively split this PR up. As we learned from @ifireball in mentor training this morning (oct 2), PRs should deliver the minimum complete feature, there's no need for example to split PRs into model changes, then view changes, then controller changes. Rather it's preferable to create a single PR for "adding users". Similarly, there's no need here to split the PR into one for installing dependencies, another for adding CSS, and a third for translating content. Should this PR therefore contain any arbitrary changes? No, an example of a follow up PR would be to update the styles, to upstream the RHDS eleventy plugin, to build a home page, to add different content, etc.

README.md Outdated Show resolved Hide resolved
@bennypowers
Copy link
Author

Per our one-on-one @ifireball, I've updated the readme. I also moved some code out of the local templates and into the plugin, so that a future PR can remove more code from this repo.

Updated preview:
beyond.zip

@Yarboa Yarboa dismissed their stale review October 3, 2022 06:33

Sorry, but i do not find the time to review this PR, hence dismissing my review to others

@EdDev
Copy link

EdDev commented Oct 3, 2022

Atomic commits

split that work

I certainly sympathize with the need to review atomic changes in this PR. It happens to be that the commits in this PR are already atomic, so you might consider reviewing them one-by-one, which will allow you to review the changes in smaller chunks.

ex post facto, it would certainly have been better to take that approach, but as it is, picking apart this PR would take about the same amount of time as the original work - there is not a lot of code here - so since the changes here are small in the grand scheme of things, and the time it would take to split the existing atomic commits into separate PRs could be better spent on other things, and since this is a volunteer project, if there is room to be a little flexible in reviewing this PR, I would very much appreciate it. I'm also available to walk through the changes one-on-one if desired.

The cost of splitting changes is always raised as a reason not to do them.
I would not advice to use it as a formal reasoning.
Eventually, if the whole package is agreed upon, the priority of fixing it is reduced, but it does make it harder to start placing things in gradually.

Content and Presention

content independent of the presentation layer

This is already the case in this PR. The content is markdown with frontmatter, the style and layout is all in the HTML templates, CSS, and eleventy config, according to Red Hat brand standards

I have noticed changes (the frontmatter?) that are not compatible with GitHub MD representation.
As such, it makes the content dependent.
I guess we could be ok with accepting a few incompatible formats, but IMO we should try and avoid them even if the cost is effecting the UI.

Maintenance

tl;dr: there are basically 0 lines of build JS, and less than 200 lines of HTML for beyond to maintain here.

That's not a tremendous amount of software to maintain, in my opinion. As well, the need for maintenance here is minimal, as software only runs in CI/CD, and there is very little security attack surface.

This is relative to the available knowledge and familiarity.
If only one person is maintaining this part, there is a risk involved.
I think that a convincing arguments may include:

  • Number of members that are familiar and able to maintain it.
  • Usage in other projects.
  • Who owns the engine, does it need upgrades from time to time, etc...

I'm also unclear what does it mean that it runs only in CI. It runs also when users are accessing the pages, no?
If there is a change from GitHub side which may require changes on the project side (e.g. upgrade, deprecation of fromats, etc) then we needs someone that knows all this to take action, otherwise the content may not be available.

Just to be clear, I am not against this initiative at all, I think it is a good step forward.
But at the same time, risk cannot be ignored and should be taken into account.

Personally, I would have preferred all non content parts be placed in a separate repo, but I understand this may not be possible with the existing tooling.

@EdDev
Copy link

EdDev commented Oct 3, 2022

@Yarboa WRT splitting the PR, we have another reason not to retroactively split this PR up. As we learned from @ifireball in mentor training this morning (oct 2), PRs should deliver the minimum complete feature, there's no need for example to split PRs into model changes, then view changes, then controller changes. Rather it's preferable to create a single PR for "adding users". Similarly, there's no need here to split the PR into one for installing dependencies, another for adding CSS, and a third for translating content. Should this PR therefore contain any arbitrary changes? No, an example of a follow up PR would be to update the styles, to upstream the RHDS eleventy plugin, to build a home page, to add different content, etc.

I think this is a an interpretation of what is minimum, complete and a feature.
One of the reasons to split work is also to allow partial changes to get in before others (e.g. preparations for a feature, which is beneficial as a stand alone as well).
On the other hand, it is valid IMO to use improvements of the content as a leverage to take the whole thing in, as long as these can be placed in a separate commit so the reviewer can focus in the review.

Generally, I would say that small PR/s have a better chance to get in than bigger PR/s.
I think this one is an example of it.
You get either little attention on big changes, or too much attention with many reviewers and opinions, which makes it harder for it to get in.
As an example, I do not feel I can review the non content parts, but I think I could review the content itself. But as this is one single PR, I cannot vote.

BTW, this is also a point to consider, you need someone who understand the changes in order to review and approve. A maintainer is suppose to approve these changes when the maintainer is willing to support it (or trusts someone to support it).

@bennypowers
Copy link
Author

I have noticed changes (the frontmatter?) that are not compatible with GitHub MD representation.

FWIW github displays the frontmatter as a table. the shortcodes will just dump raw, since gh doesn't parse liquid-style curly tags

* Number of members that are familiar and able to maintain it.

devs involved in the israel site (which i had previously offered to host this there, mind you)

* Usage in other projects.

israel site, ux.redhat.com, others

* Who owns the engine, does it need upgrades from time to time, etc...

it's open source, check out their docs site to see who's using it. doesn't need updates because it only runs in ci

I'm also unclear what does it mean that it runs only in CI. It runs also when users are accessing the pages, no?

No, it literally only runs in ci. the build generates static files, which are accessible until updated, regardless of the status of subsequent builds.

If there is a change from GitHub side which may require changes on the project side (e.g. upgrade, deprecation of fromats, etc) then we needs someone that knows all this to take action, otherwise the content may not be available.

the github action just runs the node scripts. no need to update. also, if CI breaks, the content will still be available, it just wont be updated, since the build only runs in ci and generates static files.

@heyMP
Copy link

heyMP commented Oct 3, 2022

@bennypowers As far as an 11ty build, it looks pretty straight forward and well structured. Content is segmented into distinct folders, just using headmatter to provide metadata about the page. Layouts are separated from content using the _includes layout templates. And the _data directory is the recommended 11ty way of providing dynamic structured content to layouts and plugins.

I'm not exactly sure what the history of this repo is but if it's to allow content contributors to edit markdown files in github and have those changes automatically published to a website then this looks like a good solution. We use 11ty for this exact workflow a lot and really enjoy it.

@heyMP
Copy link

heyMP commented Oct 3, 2022

The design systems team works with these technologies a lot and has a wide base of experience, we're available if there are any issues. 🙂

@bennypowers
Copy link
Author

Have we received sufficient assurances yet? Can this merge so we can iterate?

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.

5 participants