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

Move eval plugin to hls-eval-plugin #700

Merged
merged 1 commit into from
Dec 30, 2020
Merged

Conversation

tittoassini
Copy link
Contributor

Eval plugin moved to its own subpackage (as agreed with @pepeiborra).

Copy link
Member

@Ailrun Ailrun left a comment

Choose a reason for hiding this comment

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

LGTM

@pepeiborra
Copy link
Collaborator

The tests should be moved to the new package as well

@tittoassini
Copy link
Contributor Author

The tests should be moved to the new package as well

Just the testdata? Or also the test/functional/Eval.hs ?

Should plugins have totally separate test suites or should they still be integrated into the main func-test?

@pepeiborra
Copy link
Collaborator

Tests and test data should all be moved to the plugin, so that eventually the plugin can be uploaded to Hackage (and even extracted to its own GitHub repository when/if that becomes desirable)

@@ -373,7 +373,7 @@ test-suite func-test
, tasty-golden
, tasty-rerun

hs-source-dirs: test/functional plugins/tactics/src
hs-source-dirs: test/functional plugins/tactics/src plugins/hls-eval-plugin/test
Copy link
Member

Choose a reason for hiding this comment

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

I think what @pepeiborra meant is not only moving code files into inner directory but also moving (and adding) test-suite to hls-eval-plugin.cabal like tactic plugin.

@tittoassini
Copy link
Contributor Author

tittoassini commented Dec 27, 2020 via email

@pepeiborra
Copy link
Collaborator

Ouch, I think you are right, it's not currently possible to run functional tests without incurring a dependency on haskell-language-server, which would create a cycle in the Cabal graph and complicate things massively.

@jneira
Copy link
Member

jneira commented Dec 27, 2020

Yep, we have an issue about how structure tests for plugins: #576

@jneira
Copy link
Member

jneira commented Dec 27, 2020

@pepeiborra do you think we should merge this before the mergepocalypse?
@tittoassini in any case it would need a rebase for the pesky stack.yaml's

@Ailrun Ailrun merged commit 10a40cc into haskell:master Dec 30, 2020
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.

4 participants