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(README): Replace relative paths with absolute URLs #37

Merged
merged 1 commit into from
Oct 27, 2024

Conversation

polarathene
Copy link
Contributor

@polarathene polarathene commented Oct 7, 2024

The default repo page presently renders broken benchmark graph images since the relative paths are not valid from this location (you'd have to symlink the related directory I guess?).

This also makes the WASM symlink probably not that useful, unless you're utilizing it somewhere else? Potentially consider a minimal README that links to the detailed talc subdir README, along with mention of the related dedicated WASM one? May be better than the symlink approach.


I'm not sure how you're using the relative links with the symlinks, but at least via the Web UI on Github, only the README in the main repo page is rendering the symlink, while directly viewing the README.md is similar to the early linked WASM symlink which just renders the symlink path.

Side-note: You don't appear to use git tags in this repo to align with the tagged commits for your crate releases? That might be worth considering in future as a good practice (so the link I used above is to the latest repo commit).

@SFBdragon
Copy link
Owner

Just going to commit this as-is. Looks like an improvement for now. I'm going to do something else long term, most likely a repo-level README that links to the others like you suggest.

I'm going to lump that in with the other things I'm planning to do with this crate, when I'm done with these exams.

Re: git tags, I'll start using them 👍 (I haven't appreciated their utility before)

@SFBdragon SFBdragon merged commit 400252e into SFBdragon:master Oct 27, 2024
3 checks passed
@SFBdragon
Copy link
Owner

Not sure why GitHub is crediting me with authorship these commits. I guess I'm doing it wrong :/

@polarathene
Copy link
Contributor Author

polarathene commented Oct 27, 2024

I guess I'm doing it wrong :/

What merge strategy are you using? I've used squash merge for repos I maintain when PRs are received and that seems to work well.

EDIT: Looks like the default merge commit strategy 👍


A squash merge will compress the PR commit history into a single logical commit on the main branch and adds a PR reference in the commit title linking back to the PR.

I tend to prefer this approach since while it's not as nice git history diff wise, if I am using git blame it's usually within Github's own web UI, so I can always navigate to the original PR associated to that single commit for full commit and review history when more context is needed.

You seem to have the default merge commit strategy. So what this does, and a reason I don't like it is the original commit history appears on the main branch but it's interleaved commit date wise, you don't actually have a logical sequence of commits anymore, so viewing that history in the web view on Github isn't that helpful, since the longer a PR takes to merge, the more of a disconnect there is from the merge commit and the related history. Worse you can have a PR that gets new commits over days/weeks which introduces that same problem, and then if there are other PRs in parallel, when those PRs are merged you get more of that interleaved commit history.

Less of an issue with a git graph view, but personally I find that a bit messy to look at or you have to filter the branches out, at which point just go with squash commits anyway? Depends on you goals and workflow really.

There's a third option, rebase merge which is alright. Before you merge, all commits are rebased to the latest base branch commit, so they'll all appear grouped together and be appended onto the branch. This is alright if you want to retain the individual logical commits in the proper branch history, but if you do this it mostly makes sense to have proper housekeeping by spending time tailoring commits to be meaningful and have commit messages with relevant context.

Squash commits work great for third-party contributions, reviews can be iterative, you're less fussed about the commit history of the PR branch getting messy as you're more focused on the final result and review itself. Less friction on the contributor too. When you merge the PR, you may want to adjust the commit title for the squashed commit, and Github will present the commit message as all commit titles + messages of the PR combined, so I just have a quick pass and trim out anything redundant, or copy/paste the PR description/summary, knowing that the PR will be referenced by the squashed commit should more context be needed (it's often rare).


Anyway, here's what your merge commit looks like, you can see since this is only a single commit PR, we both have the same commit title, just different dates and authorship. If you view this in a git graph you'll see my commit is from this separate branch that your merge commit "merges" in diff wise.

image

I'm not fussed about the credit, but if this was a squash merge it'd be a commit like mine but where your merge commit is, it'll attribute me as the author and you as the committer IIRC. You can see the docker-mailserver commit history if you'd like to see what squash commit looks like in comparison.

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