Skip to content

Conversation

mjang
Copy link
Contributor

@mjang mjang commented Oct 18, 2024

  • Include go files to support F5/NGINX Hugo theme
  • Add files which support make for different build types

Proposed changes

Detailed info on how users can build their own docs, based on the standard NGINX Open Source CONTRIBUTING.md. file. However, I've done this in CONTRIBUTING_DOCS.md, as our template for new open source repos has its own CONTRIBUTING.md file.

I've also included files needed to build docs locally, such as Makefile, go.*, etc. In my tests, users can now run the make commands described in CONTRIBUTING_DOCS.md.

This is sort of a meshing of "README"-type files our current open docs repos and the template repository, including the .github/pull_request_template.md file.

Checklist

Before creating a PR, run through this checklist and mark each as complete:

@mjang mjang requested a review from a team as a code owner October 18, 2024 18:56
@mjang mjang self-assigned this Oct 18, 2024
@mjang mjang force-pushed the doc-build-instructions branch 3 times, most recently from a48a929 to 404fc83 Compare October 18, 2024 19:49
@mjang mjang requested review from ADubhlaoich and removed request for a team October 18, 2024 19:58
@mjang mjang force-pushed the doc-build-instructions branch 2 times, most recently from 721ddfe to 43a0372 Compare October 18, 2024 21:11
Copy link
Contributor

@ADubhlaoich ADubhlaoich left a comment

Choose a reason for hiding this comment

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

LGTM. Some minor comments, tiny LOGAF.

Comment on lines 22 to +29
- [ ] I have read the [contributing guidelines](/CONTRIBUTING.md).
- [ ] I have signed the [F5 Contributor License Agreement (CLA)](https://github.com/f5/.github/blob/main/CLA/cla-markdown.md).
- [ ] If applicable, I have added tests that prove my fix is effective or that my feature works.
- [ ] If applicable, I have checked that any relevant tests pass after adding my changes.
- [ ] I have updated any relevant documentation ([`README.md`](/README.md) and [`CHANGELOG.md`](/CHANGELOG.md)).
- [ ] I have rebased my branch onto main
- [ ] I will ensure my PR is targeting the main branch and pulling from my branch from my own fork
- [ ] If the change involves:
Copy link
Contributor

Choose a reason for hiding this comment

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

IA thing: I'd consider re-ordering these based on how frequently someone will come in contact with them. I often check the box for contributing in PRs despite being well aware of the policy: the F5 CLA is only required on the user's first interaction with a repo, and the bot enforces it as a requirement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ADubhlaoich I hear you. I've tried to "respect" the existing checklist order from various NGINX repos. I'm personally good with the order, since contributing guidelines and the CLA are prerequisites.

- Release notes
- Tutorial

## How to format docs
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## How to format docs
## How to format documentation

Comment on lines +79 to +95
### How to format internal links

Internal links should use Hugo [ref and relref shortcodes](https://gohugo.io/content-management/cross-references/).

- Although file extensions are optional for Hugo, we include them as best practice for page anchors.
- Relative paths are preferred, but just the filename is permissible.
- Paths without a leading forward slash (`/`) are first resolved relative to the current page, then the remainder of the website.

Here are two examples:

```md
To install <software>, refer to the [installation instructions]({{< ref "install.md" >}}).
To install <integation>, refer to the [integration instructions]({{< relref "/integration/thing.md#section" >}}).
```

Copy link
Contributor

Choose a reason for hiding this comment

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

No feedback, really: I've taken to just using ref since the way we use relref is redundant.

Having the full, explicit path is much clearer, especially when you're making IA changes (Which I seem to do often).

I'm not sure if this would be worth proposing as a convention going forward.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I personally agree, but think we should come to a consensus as a documentation group before making that kind of change.

> **Important**: We have strict guidelines regarding the use of images in our documentation. Make sure that you keep the number of images to a minimum and that they are relevant to the content. Review the guidelines in our [style guide](/templates/style-guide.md#guidelines-for-screenshots).
### How to use Hugo shortcodes
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be nice to include our new ghcode shortcode here.

Every change to this file has to be reflected in all of the other counterparts, but I am inclined to be lazy about updating the others on the basis that this will eventually be the sole documentation repo for NGINX projects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks cool! I don't see ghcode at least in our current repositories.

- Include go files to support F5/NGINX Hugo theme
- Add files which support `make` for different build types
- Set up a compatible code request template
- Update .gitignore, go files
- Include min 2 approvals for potential security issues
- Add detailed "which repo" instructions to F5-NGINX-team-notes
@mjang mjang force-pushed the doc-build-instructions branch from 114a4ad to 0d9f663 Compare November 20, 2024 22:19
@mjang
Copy link
Contributor Author

mjang commented Nov 20, 2024

Notes:

  • It's OK for the F5/CLA check to fail, since this is not yet an open source repo
  • The CODEOWNERS file suggests that the groups are in error. It will be in error until we move this repo to the nginx org (which suggests our next step).

I'm therefore proceeding with merge.

@mjang mjang merged commit 4915355 into main Nov 20, 2024
1 check failed
@mjang mjang deleted the doc-build-instructions branch December 11, 2024 13:37
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