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

Support for new command #2202

Merged
merged 14 commits into from Jun 20, 2023
Merged

Support for new command #2202

merged 14 commits into from Jun 20, 2023

Conversation

zetashift
Copy link
Contributor

Adds a new command to scala-cli to enable creating projects through giter8.

Resolves #19

New to the scala-cli codebase, so all feedback welcome as I have no real idea if I'm heading in the right direction.
I'll add docs when the code is in a better shape!

Adds a new command to scala-cli to enable creating projects through giter8
@lwronski
Copy link
Contributor

It would also be beneficial to add some integration tests. You can create a new class NewTests in integration test, similar to FmtTests.

@zetashift
Copy link
Contributor Author

zetashift commented Jun 14, 2023

It would also be beneficial to add some integration tests. You can create a new class NewTests in integration test, similar to FmtTests.

Currently trying to add integration tests, but kinda lost in what kind of template to expect.

package scala.cli.integration

class NewTests extends ScalaCliSuite {
  
}

The scalafmt integrations seem quite straightforward, but for the giter8 dep I need some "default" scala-cli giter8 template no? And should this live on my github or can we update (for example) this one: https://github.com/VirtuslabRnD/scala-cli.g8 ?

@lwronski
Copy link
Contributor

It would also be beneficial to add some integration tests. You can create a new class NewTests in integration test, similar to FmtTests.

Currently trying to add integration tests, but kinda lost in what kind of template to expect.

package scala.cli.integration

class NewTests extends ScalaCliSuite {
  
}

The scalafmt integrations seem quite straightforward, but for the giter8 dep I need some "default" scala-cli giter8 template no? And should this live on my github or can we update (for example) this one: https://github.com/VirtuslabRnD/scala-cli.g8 ?

I think it is the default repository that scala-cli wants to maintain. So let's use them in the integration test.

@zetashift zetashift marked this pull request as ready for review June 18, 2023 15:37
@zetashift
Copy link
Contributor Author

I think it is the default repository that scala-cli wants to maintain. So let's use them in the integration test.

No idea why I couldn't find this, thank you! :D

zetashift and others added 3 commits June 19, 2023 09:43
Co-authored-by: Łukasz Wroński <46607934+lwronski@users.noreply.github.com>
@zetashift
Copy link
Contributor Author

CI is reporting:

scala.cli.integration.SparkTests213:
  + run spark 3.3 standalone 9.271s
  + run spark spark-submit args 8.239s
scala.cli.integration.ExportJsonTestsDefault:
scala.cli.integration.DefaultFileTests:
1 targets failed
integration.test.test 1 tests failed: 
  scala.cli.integration.PackageTests213.simple script scala.cli.integration.PackageTests213.simple script
Error: Process completed with exit code 1.

I am quite sure I am not touching this part of the code:

@lwronski
Copy link
Contributor

CI is reporting:

scala.cli.integration.SparkTests213:
  + run spark 3.3 standalone 9.271s
  + run spark spark-submit args 8.239s
scala.cli.integration.ExportJsonTestsDefault:
scala.cli.integration.DefaultFileTests:
1 targets failed
integration.test.test 1 tests failed: 
  scala.cli.integration.PackageTests213.simple script scala.cli.integration.PackageTests213.simple script
Error: Process completed with exit code 1.

I am quite sure I am not touching this part of the code:

Not really, it throws some timeout. I will re-run CI.

@zetashift
Copy link
Contributor Author

Not really, it throws some timeout. I will re-run CI.

Ohhh didn't see that 🙈 , thank you :D.

@bishabosha
Copy link
Contributor

bishabosha commented Jun 19, 2023

does this also create the .bsp/scala-cli.json file so that IDE import works right after? - would be good to not require another step from the user

@zetashift
Copy link
Contributor Author

does this also create the .bsp/scala-cli.json file so that IDE import works right after? - would be good to not require another step from the user

Currently the command does not do this. I don't mind adding it!
A consideration:

  • There is also nothing stopping one from instantiating a g8 template that targets a sbt or mill project, so maybe that option should be a flag?

@lwronski
Copy link
Contributor

does this also create the .bsp/scala-cli.json file so that IDE import works right after? - would be good to not require another step from the user

No, it doesn't generate .bsp automatically, but it would be a good idea for Scala CLI to do so. I think this idea could be implemented in a separate pull request.

@zetashift
Copy link
Contributor Author

I think this idea could be implemented in a separate pull request.

Yeah, sounds good to me.

Copy link
Contributor

@lwronski lwronski left a comment

Choose a reason for hiding this comment

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

LGTM @zetashift Thanks for your contribution

@lwronski
Copy link
Contributor

I think this idea could be implemented in a separate pull request.

Yeah, sounds good to me.

To keep track of this idea, I have created an issue for it - #2230

@lwronski lwronski merged commit 239a604 into VirtusLab:main Jun 20, 2023
38 checks passed
@zetashift zetashift deleted the add-new-command branch June 20, 2023 14:30
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.

Add new command?
5 participants