Skip to content

enhance www/README.md #912

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

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

enhance www/README.md #912

wants to merge 5 commits into from

Conversation

hubyrod
Copy link
Contributor

@hubyrod hubyrod commented May 22, 2025

No description provided.

@hubyrod hubyrod requested a review from jberdine May 22, 2025 15:59
@hubyrod hubyrod marked this pull request as ready for review May 23, 2025 07:02
@hubyrod hubyrod force-pushed the www_readme branch 4 times, most recently from 3e5deac to b6a4a7d Compare May 23, 2025 12:41
@jberdine
Copy link
Contributor

I think something got slightly messed up with a rebase or something, some of the changes for resolved comments are not applied, and there is a duplicate ```bash line.

@hubyrod hubyrod requested a review from jberdine May 30, 2025 19:11
@jberdine
Copy link
Contributor

jberdine commented Jun 3, 2025

Did you check that things are in the desired state after the rebase recovery? It looks to me that the comments marked as resolved have still been undone in the current state of this branch.

www/README.md Outdated
git clean -Xdf # the actual cleaning
```

### What if the hash is not in the commit message suggested along the process ?
Copy link
Contributor

Choose a reason for hiding this comment

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

At least in my experience, the observation that something is wrong is that git push asks for --force, which we shouldn't do. I don't actually understand the question of this FAQ, what is "the hash" it is referring to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What has happened in the past, is that the hash in the commit message is not the expected hash.
May be not well phrased. Let me suggest something different.

@hubyrod hubyrod force-pushed the www_readme branch 2 times, most recently from f2f9184 to f3166e3 Compare June 4, 2025 08:31
[here](https://typedoc.org/guides/tags/).
The commit hash in the last line should be the hash of your commit in the skip repository.

> **Note**: If `make docs-publish` is run from a commit that is not (an ancestor of) `main`, the docs_site commit messages will contain incorrect commit hashes.
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
> **Note**: If `make docs-publish` is run from a commit that is not (an ancestor of) `main`, the docs_site commit messages will contain incorrect commit hashes.
> **Note**: If `make docs-publish` is run from a commit that is not (an ancestor of) `main`, the docs_site commit messages will contain unreproducible commit hashes.


### Publishing Documentation

Before, commit your changes and send them for review
Copy link
Contributor

Choose a reason for hiding this comment

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

Up to you, but I would suggest:

Suggested change
Before, commit your changes and send them for review
First commit your changes and send them for review

```
This will suggest to run:

When reviewed and merged, you are ready to publish documentation to the live site:
Copy link
Contributor

Choose a reason for hiding this comment

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

Up to you, but I would suggest:

Suggested change
When reviewed and merged, you are ready to publish documentation to the live site:
Once reviewed and merged, you are ready to publish documentation to the live site:

Comment on lines +76 to +88
### What if my `docs_site` is not on the `main` branch ?
Clean up your repository is good idea at this point and then start the process again:
```bash
git pull --rebase # ensure that your local docs_site submodule is up-to-date
git clean -Xdn # -n for a dry-run and double-check what's to be cleaned out
git clean -Xdf # the actual cleaning
```

### What if the hash in the suggested commit message is not the hash of your commit?
You can get it from the skip repository:
```bash
git rev-parse --short HEAD # supposingly it is the last commit of the branch
```
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't mean to be difficult, but I still don't understand. I'm not sure if two potential issues are being mixed up. The two issues I am aware of are:

  1. The local docs_site submodule is behind the upstream one.
  2. There is one or more local commits that have not been merged into the upstream main branch.

For 1., the symptom is that the git push suggested by make docs-publish complains about needing --force. In this situation, we do not want to force push, but should instead update the docs_site submodule and regenerate the docs and try to publish again.

For 2., the symptom is that git merge-base HEAD origin/main is not equal to HEAD, which will lead to the commit message suggested by make docs-publish to contain a <commit> that is only local. In this situation we do not want to change that commit message, we want to make sure that the HEAD commit that we publish is an ancestor of origin/main.

So there are potential confusions in these FAQs:

  1. The solution for docs_site being out of date is to update the submodule. The cleaning is independent of docs, and is just a general issue due to missing dependencies in the build system that are sometimes encountered when updating across a bootstrap of the skiplang compiler or runtime system.
  2. The FAQ is not clear. The hash in the suggested commit message is correct, it just might be that the commit is not merged into origin/main. The FAQ makes it sound like the commit message should be changed, but that is not the right action.

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