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

Refactor #5

Merged
merged 20 commits into from
Mar 27, 2023
Merged

Refactor #5

merged 20 commits into from
Mar 27, 2023

Conversation

lpotthast
Copy link
Collaborator

@lpotthast lpotthast commented Mar 27, 2023

Hi, now to the PR I promised. I does a lot of things by now... Let me try to explain most of them (if I remember them all). Note: I incorporated all your last commits since I forked.

  • The builder is no longer a build script. Working with them is hard, and in this case, it must not even run automatically, we would actually much rather want to trigger the build on our own.

  • Therefore, the repository now hosts two completely separate crates, the builder binary crate and the actual library. Their readmes reflect that.

  • One can now cd build and call cargo run to let the builder create or update the library. Use cargo run -- --clean to force package re-downloads.

  • The builder not being a build script also makes observability much easier (build scripts usually swallow all output to stdout and require workarounds...).

  • The tracing library is now used throughout the builder, revealing what is going on with what data at most times.

  • As there is a lot of I/O, I updated the builder to run on a Tokio runtime, using tokio::fs instead of std::fs throughout.

  • Each icon package is managed in parallel, with minimum stuff before or after that.

  • All packages are now described in code.

  • All packages are defined with specific versions. Either by using a git tag or commit hash. This makes the generation much more stable.

  • Packages are no longer added as submodules. Working with submodules is hard enough manually, automatically creating and removing them is not that great and not really required.

  • Git icon packages are now simply cloned to the /downloads directory, performing a checkout for the expected git tag or commit ref.

  • Working with thousands of files puts a lot of strain on many parts of the system (rust-analyzer, IDE, Git, cargo, ...). And of course, generating them all is slow.

  • Therefore: All icons are now generated inside a single module, without any module nesting for categories, reducing the amount of files generated to the number of icon packages involved. A build without downloads should take less then 10 seconds.

  • Note: It currently generates a few "unused use" warnings. They can be resolved with Allow component decl without use leptos::* in scope leptos-rs/leptos#748

  • All categories are appended to the icons base name, making them not overlap. We could(should?) also verify that this does not happen with a different data structure / a bit more code.

  • All icons share a feature and component name, making it as easy as possible for any given user to activate and use them.

  • This means hat icons are prefixed by their package short name. This also fixes any problems with icons starting with numeric characters.

  • Many smaller or larger architectural changes, making programming errors less likely by leveraging the type system a bit more.

  • The README.md of the library is generated, including the table of icon packages and their specific version.

  • An ICONS.md file is generated, including all icon names from each icon package.

  • NOTE: It will always be a breaking change for leptos-icons if any icon-package is updated (version increased or decreased) and that change was itself breaking.
    To reduce version changes for any given user, onw might publish separate crates like leptos-icons-fa, leptos-icons-bs, ...

@carloskiki
Copy link
Owner

carloskiki commented Mar 27, 2023

Wow, this is astonishing!

I'm pretty overwhelmed by this pr, but these are all great changes and I will surely merge them shortly.

I was just wondering why you chose the src/module/mod.rs over the src/module.rs convention?

@carloskiki carloskiki merged commit 34933e3 into carloskiki:main Mar 27, 2023
@lpotthast
Copy link
Collaborator Author

Hi, the leptos-component functions i would leave non snake-case, as it is the UI / Leptos default. To the module convention: I think it's more or less a personal preference. Feel free to change anything. I just like the reduced "visual distance between module related files when looking at a file tree". With a mod.rs file, everything belonging to module "x" is contained in directory "x". I think it is a bit cleaner. The file simply be named "mod.rs" is a downside though... If there would be a "x.rs", defining the module, that might be, in a bigger project, be located "further away" the directory. Either way is totally fine :)

carloskiki added a commit that referenced this pull request Oct 8, 2023
* Add one codecov

* Merge another codecov

* Merge another codecov

* Merge another codecov

* Merge another codecov

* Place codecov config under .github

* Add (only) ASAN workflow

* Add first coverage workflow

* Merge another coverage.yml

* Merge another coverage.yml

* Add first features workflow

* Merge another features workflow

* Merge another features workflow

* Merge another features workflow

* Add (only) loom workflow

* Add (only) LSAN workflow

* Add first minial workflow

* Add (only) miri workflow

* Add first msrv workflow

* Merge another msrv workflow

* Merge another msrv workflow

* Merge another msrv workflow

* Add (only) no-std workflow

* Add first os-check workflow

* Merge another os-check workflow

* Add first style workflow

* Merge another style workflow

* Merge another style workflow

* Add first test workflow

* Merge another test workflow

* Merge another test workflow

* Merge another test workflow

* Make everything use checkout@v3

* Standardize on 'main' as branch name

* Missed a submodule checkout

* Add TODOs from twitter thread

* Practice what you preach

* mv github .github

This should make it possible to have rust-ci-conf as a remote you merge
from.

* Merge safety workflows

* Catch upcoming deprecations

* More concise name for scheduled jobs

* Allow examples and binaries to require features

* Use dependabot, but only for major versions

* ignore is a list

* Notify if actions themselves are outdated

* Bump codecov/codecov-action from 2 to 3

Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 2 to 3.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md)
- [Commits](codecov/codecov-action@v2...v3)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

* Move to maintained rust installer

See actions-rs/toolchain#216

* Fix install message for msrv

* Get rid of most actions-rs bits

Given that that project is unmaintained.

actions-rs/toolchain#216

* Minimal token permissions

See tokio-rs/tokio#5072

* Remove -Zmiri-tag-raw-pointers as it's now default

* Unbreak cargo hack for non-libraries (#4)

* Add action to run doctest. (#3)

`cargo test --all-features` does not run doc-tests. For more information
see rust-lang/cargo#6669.

* chore: automatically cancel superseded Actions runs (#5)

* [sanity] More robust injection of opt-level 1 (#9)

Fixes #8

* Quote MSRV version to avoid float parsing (#11)

Put 1.70 in there (for instance if you want to pin against OnceLock stabilizing) and it will actually test 1.7 as it appears github auto converts this to a float?

Putting in quotes seems to do the right thing here

* Install Openssl for Windows (#12)

* Don't install OpenSSL on Windows by default

* Bump actions/checkout from 3 to 4 (#13)

Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3...v4)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Jon Gjengset <jon@thesquareplanet.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Tudyx <56633664+Tudyx@users.noreply.github.com>
Co-authored-by: Burkhard Mittelbach <wasabi37a@googlemail.com>
Co-authored-by: Simen Bekkhus <sbekkhus91@gmail.com>
Co-authored-by: James Chacon <chacon.james@gmail.com>
Co-authored-by: Rod Elias <rodiney@gmail.com>
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