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

Remove build artifacts from the repo #41

Merged
merged 16 commits into from
Mar 27, 2024
Merged

Remove build artifacts from the repo #41

merged 16 commits into from
Mar 27, 2024

Conversation

gabalafou
Copy link
Contributor

@gabalafou gabalafou commented Feb 20, 2024

This PR removes all of the generated HTML and CSS from the repo and refactors the build scripts.

There is perhaps an argument for committing built examples to the repo. That way a user can open the demo page from their own local repo into their browser without having to execute any build command first. But we can add that later if we want.

My goal is to make the repo easier to understand by stripping it of everything but READMEs, config files, source code, scripts, and tests.

My ultimate goal is to simplify and automate the process of updating or adding a theme. This PR helps that goal by removing steps 2, 3, and 4 from the list of steps in issue #34. The trade-off that this PR makes to achieve that goal is that the docs/index.html file can no longer be opened locally from the repo and just work (to make it work locally, developers will need to run the render_html command). Subsequent PRs will automate steps 5 and 6, so that updating a theme will involve the following steps:

  1. Update theme's style.py
  2. Run a script to update the theme's README (color contrast table and screenshot)
  3. git commit

Copy link

github-actions bot commented Feb 20, 2024

PR Preview Action v1.2.0
🚀 Deployed preview to https://Quansight-Labs.github.io/accessible-pygments/pr-preview/pr-41/
on branch gh-pages at 2024-03-27 12:10 UTC

Copy link
Contributor Author

@gabalafou gabalafou left a comment

Choose a reason for hiding this comment

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

Done self-reviewing. Hopefully we can test the GitHub Pages action before merging this?

.github/workflows/preview-pr.yml Show resolved Hide resolved
a11y_pygments/utils/utils.py Show resolved Hide resolved
docs/index.css Outdated
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 just felt like moving these styles out of index.html

docs/index.html Show resolved Hide resolved
<option>gotthard-dark</option>
<option>blinds-light</option>
<option>blinds-dark</option>
<option>greative</option>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will need to automate this list of themes somehow. Maybe we'll make this index.html file a Jinja template?

docs/index.html Show resolved Hide resolved
docs/index.js Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
test/render_html.py Show resolved Hide resolved
@gabalafou
Copy link
Contributor Author

gabalafou commented Feb 21, 2024

Do we know why PR-preview (GitHub action) doesn't work?

@trallard trallard added the type: enhancement 💅🏼 New feature or request label Feb 21, 2024
gabalafou added a commit that referenced this pull request Mar 26, 2024
Copy link
Member

@trallard trallard left a comment

Choose a reason for hiding this comment

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

Will need to automate this list of themes somehow. Maybe we'll make this index.html file a Jinja template?

Is this being done in other PR? think I saw another one with jinja templates

Copy link
Member

@trallard trallard left a comment

Choose a reason for hiding this comment

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

Thanks @gabalafou, this looks great and helps make this repo a lot more maintainable.
Made a couple of small changes (pushed) and suggestions, if you are happy with these we can merge right away.

@gabalafou
Copy link
Contributor Author

Is this being done in other PR? think I saw another one with jinja templates

No, this is still an open problem. The Jinja template in the other PRs is used to generate the theme READMEs, but not the demo index.html page.

@gabalafou gabalafou merged commit cd94516 into main Mar 27, 2024
6 checks passed
@gabalafou gabalafou deleted the simplify-repo branch March 27, 2024 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants