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

Scalafix followups: Move doc specs to doc/jdoc package #26045

Open
1 of 3 tasks
chbatey opened this issue Dec 5, 2018 · 6 comments
Open
1 of 3 tasks

Scalafix followups: Move doc specs to doc/jdoc package #26045

chbatey opened this issue Dec 5, 2018 · 6 comments
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted help wanted Issues that the core team will likely not have time to work on t:docs

Comments

@chbatey
Copy link
Member

chbatey commented Dec 5, 2018

#26019 (review) introduced scalafix to remove unused imports. It is not enabled on compile and can remove imports that are included in specs for docs.

To followup:

  • Ignore files in doc/jdoc package
  • Move any doc specs to the doc for Scala and jdoc package so that imports are necessary e.g. FlowZipSpec is in the akka.stream.scaladsl and is used for docs. The unit test parts should remain but the docs part should be in a new file in the docs package do that imports from akka.stream.scaladsl are required
  • Test how fast it is to enable on compile and decide if we want it enabled by default
@chbatey chbatey added 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:docs labels Dec 5, 2018
@chbatey chbatey added the help wanted Issues that the core team will likely not have time to work on label Dec 5, 2018
@He-Pin
Copy link
Member

He-Pin commented Dec 5, 2018

I think Ignore files in doc/jdoc package is already done
Sorry, actually it not done yet: #26084

@dwijnand
Copy link
Member

dwijnand commented Dec 5, 2018

Why is the ignore files part necessary? Once they're out of their owning package aren't they safe to be kept clean by Scalafix?

@He-Pin
Copy link
Member

He-Pin commented Dec 8, 2018

best to defer after :#26043

@He-Pin
Copy link
Member

He-Pin commented Dec 9, 2018

without

[success] Total time: 439 s, completed 2018-12-10 3:45:26

with

[success] Total time: 643 s, completed 2018-12-10 5:25:08

@chbatey I think it slowed a lot.

@He-Pin
Copy link
Member

He-Pin commented Dec 11, 2018

another way to protect is

//#zip-with-index
import akka.stream.scaladsl.Source // scalafix:ok
//#zip-with-index

as @marcelocenerine mentioned at :scalacenter/scalafix#918

@raboof
Copy link
Member

raboof commented Dec 14, 2018

That would be possible, but easy to forget. Also, moving those tests to a separate package protects against forgetting imports or accidentally depending on non-public API's in documentation snippets. So it still seems worth doing.

Good to know though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted help wanted Issues that the core team will likely not have time to work on t:docs
Projects
None yet
Development

No branches or pull requests

4 participants