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

More refactor #225

Merged
merged 2 commits into from
Apr 16, 2021
Merged

More refactor #225

merged 2 commits into from
Apr 16, 2021

Conversation

lebensterben
Copy link
Member

  • Major changes in lychee-lib::filter module:
    • Fields in Excludes except the RegexSet is now moved to Filter.
    • Filter contains Option<Excludes> and Option<Includes>, which are
      wrapper struct of RegexSet instead of Option<RegexSet>. As a result
      the code now looks cleaner.
    • Factored out some filtering logics to dedicated functions.
      • It's possible to write tests for those functions in addition to tests
        for the Filter struct.
    • Added docs to Filter::is_excluded and reorgnized the code.
  • placed derive_builder by typed_builder:
    • The internal interface very ugly, as admitted by the author, but we no
      longer have nested Options like before.
    • As a result, the Client building is much easier to read.
    • Main benefit of typed_builder is, the arguments feeded to builder is
      checked at compile time instead of run-time.
  • Fixed a bug in lychee::tests::usage and lychee-lib::stats::test.
    • Now it will clear environment variable which would otherwise cause an
      issue if GITHUB_TOKEN is set.
  • Updated dependencies.

- Major changes in `lychee-lib::filter` module:
  - Fields in `Excludes` except the `RegexSet` is now moved to `Filter`.
  - `Filter` contains `Option<Excludes>` and `Option<Includes>`, which are
    wrapper struct of `RegexSet` instead of `Option<RegexSet>`. As a result
    the code now looks cleaner.
  - Factored out some filtering logics to dedicated functions.
    - It's possible to write tests for those functions in addition to tests
      for the `Filter` struct.
  - Added docs to `Filter::is_excluded` and reorgnized the code.
- placed `derive_builder` by `typed_builder`:
  - The internal interface very ugly, as admitted by the author, but we no
    longer have nested `Option`s like before.
  - As a result, the `Client` building is much easier to read.
  - Main benefit of `typed_builder` is, the arguments feeded to builder is
    checked at compile time instead of run-time.
- Fixed a bug in `lychee::tests::usage` and `lychee-lib::stats::test`.
  - Now it will clear environment variable which would otherwise cause an
    issue if `GITHUB_TOKEN` is set.
- Updated dependencies.
@lebensterben
Copy link
Member Author

Well #222 was closed since I've renamed the branch to work with git-flow.

@lebensterben lebensterben mentioned this pull request Apr 15, 2021
@mre
Copy link
Member

mre commented Apr 15, 2021

lgtm. There is one final error in https://github.com/lycheeverse/lychee/pull/225/checks?check_run_id=2354928844 that should be an easy fix.

@lebensterben
Copy link
Member Author

You can build it locally, since cargo test passes.
cargo publish instead look for the package from registry, which is the one published yesterday. So it complains about not finding the function.

Again we need to publish the lychee-lib first.

P.S. And I haven't changed the version number. It could be v0.7.0.alpha since the API changes.

@mre
Copy link
Member

mre commented Apr 16, 2021

Works locally. Let's merge it. 😄

@mre mre merged commit f64213d into lycheeverse:master Apr 16, 2021
@dblock dblock mentioned this pull request Sep 1, 2021
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