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

Add fix command #1627

Closed
wants to merge 3 commits into from
Closed

Add fix command #1627

wants to merge 3 commits into from

Conversation

wleczny
Copy link
Contributor

@wleczny wleczny commented Nov 30, 2022

No description provided.

@wleczny wleczny marked this pull request as ready for review November 30, 2022 23:04
@wleczny wleczny force-pushed the fix-command branch 2 times, most recently from 2209be9 to ebe0936 Compare November 30, 2022 23:07
Copy link
Contributor

@Gedochao Gedochao left a comment

Choose a reason for hiding this comment

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

Can we add some integration tests?

website/docs/commands/fix.md Outdated Show resolved Hide resolved
website/docs/commands/fix.md Outdated Show resolved Hide resolved
website/docs/commands/fix.md Outdated Show resolved Hide resolved
@Gedochao
Copy link
Contributor

I think this won't work with markdown inputs for now, although that can be fixed in a separate PR

@wleczny wleczny force-pushed the fix-command branch 2 times, most recently from 9f89023 to e85deed Compare March 6, 2023 17:03
@wleczny wleczny changed the title Add fix and migrate-directives commands Add fix command Mar 6, 2023
@wleczny wleczny force-pushed the fix-command branch 3 times, most recently from ffea39b to 7250d96 Compare March 6, 2023 17:20
Copy link
Contributor

Choose a reason for hiding this comment

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

Some example with directives spread between multiple files would be useful.
An example output project.scala could then be included, plus there'd be no reason to ignore the '''bash snippets in docs-tests


import com.eed3si9n.expecty.Expecty.expect

class FixTests extends ScalaCliSuite {
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if I now have directives in different scopes? For example main directives in Hello.scala and test directives in Hello.test.scala?

We should make sure that we don't accidentally broke someone's Scala CLI configuration.

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be also tested with Scala.Js and Scala native scopes.

@Gedochao
Copy link
Contributor

Closing this, we'll make a new attempt soon.
to be tracked under #2104

@Gedochao Gedochao closed this May 10, 2023
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.

None yet

3 participants