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

Clarify how to run this locally #1

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

Conversation

ross-spencer
Copy link
Member

@ross-spencer ross-spencer commented Nov 5, 2024

Updating the README based on giving this a whirl. The output is really nice.

NB. one line has been removed here because it was unclear the intention so it might need adding in another commit:

After building, you will see `index.html` under the build folder. The contents of this file are then to be copied and pasted over the contents of `/static/archive-viewer.html` in the [Explorer](https://github.com/orcfax/explorer.orcfax.io/blob/main/static/archive-viewer.html).

The last link is a 404 and so needs to be corrected either way.

Pre-commit tooling

We have some tooling that should be shared across repositories for consistent formatting of things like Markdown. This means we all contribute following the same foundation making it easier and reducing cognitive load. Probably especially important for public contributions so that they follow a consistent standard.

Each commit is atomic, two specifically of interest when reviewing:

Adding the tooling: 4b3d3e7

Formatting the README: a127aba

There is a .github workflow that will be triggered for pull-requests as well so that helps catch issues coming in.

Prettier

For markdown, prettier is an option, the glossary is a good reference point:

We have one recipe for it, but I couldn't make it work with this repository. I think maybe because prettier is already used somewhere?

# Prettify markdown
prettier:
  prettier ./**/*.md --write --print-width 80 --prose-wrap always
  for x in "{{md}}"; do markdownlint --fix $x; done

I am not sure if you want to take this one and add it? It can always be added in future.

@gchartier
Copy link
Member

gchartier commented Nov 5, 2024

Thanks for the review, will take a look into this.

NB. one line has been removed here because it was unclear the intention so it might need adding in another commit:

That line should probably be under a new section called Deployment because that's essentially how deployment is done ie. copying over the build output to the static asset in the explorer. Will add this with more clarity.

As for the formatting configs, I want to make sure running pnpm run format and pnpm run lint will pick up these configs. And yes, I am already using prettier so will likely just need to add in the markdown config into my existing setup.

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