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 dependency on Pkg #100

Closed
giordano opened this issue Sep 4, 2024 · 1 comment · Fixed by #115
Closed

Remove dependency on Pkg #100

giordano opened this issue Sep 4, 2024 · 1 comment · Fixed by #115
Assignees

Comments

@giordano
Copy link

giordano commented Sep 4, 2024

I think there are two problems here:

  • the main package requires Pkg:
    Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
    while not using it anywhere in the source code. This should be solved by just removing that line in Project.toml
  • the tests use Pkg to run the tests of upstream packages:

    Herb.jl/test/runtests.jl

    Lines 15 to 26 in 13e8b88

    println("\n--- HerbConstraints tests ---")
    Pkg.test("HerbConstraints")
    println("\n--- HerbCore tests ---")
    Pkg.test("HerbCore")
    println("\n--- HerbInterpret tests ---")
    Pkg.test("HerbInterpret")
    println("\n--- HerbGrammar tests ---")
    Pkg.test("HerbGrammar")
    println("\n--- HerbSearch tests ---")
    Pkg.test("HerbSearch")
    println("\n--- HerbSpecification tests ---")
    Pkg.test("HerbSpecification")
    This package looks like a metapackage which pulls together multiple packages, which is fine, but I'm not sure just running upstream packages test is a good approach, since they are already tested individually. Can there be integration tests, for example based on the tutorials in the docs, to make sure they keep working?
@ReubenJ
Copy link
Member

ReubenJ commented Sep 5, 2024

Thanks for the suggestion, @giordano! Removing the dependency on Pkg definitely makes sense. I had started adding workflows for doctests to the individual repositories in #63, and I could also see doctests on the tutorials replacing the individual package tests in this meta package. If it's as easy as copying similar changes over from what I was working on in #63, I can get this updated soon. Otherwise, we'll put it on our to-do list to address soon.

Thanks again for the suggestion!

@ReubenJ ReubenJ self-assigned this Nov 14, 2024
@ReubenJ ReubenJ linked a pull request Nov 14, 2024 that will close this issue
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 a pull request may close this issue.

2 participants