Skip to content

Move more packages into dom #545

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

Closed
6 of 14 tasks
japgolly opened this issue Sep 3, 2021 · 20 comments · Fixed by #588
Closed
6 of 14 tasks

Move more packages into dom #545

japgolly opened this issue Sep 3, 2021 · 20 comments · Fixed by #588
Milestone

Comments

@japgolly
Copy link
Contributor

japgolly commented Sep 3, 2021

Move the following into dom:

  • beacon
  • crypto
  • deviceorientation
  • domparser
  • gamepad
  • mediastream
  • permissions
  • push
  • serviceworkers
  • sharedworkers
  • storage
  • webrtc

This should leave us with the following packages:

  • intl
  • webgl
@japgolly
Copy link
Contributor Author

japgolly commented Sep 3, 2021

Decided to count LoC:

for f in \
  src/main/scala/org/scalajs/dom/crypto \
  src/main/scala/org/scalajs/dom/experimental/beacon \
  src/main/scala/org/scalajs/dom/experimental/cachestorage \
  src/main/scala/org/scalajs/dom/experimental/deviceorientation \
  src/main/scala/org/scalajs/dom/experimental/domparser \
  src/main/scala/org/scalajs/dom/experimental/gamepad \
  src/main/scala/org/scalajs/dom/experimental/intl \
  src/main/scala/org/scalajs/dom/experimental/mediastream \
  src/main/scala/org/scalajs/dom/experimental/permissions \
  src/main/scala/org/scalajs/dom/experimental/push \
  src/main/scala/org/scalajs/dom/experimental/serviceworkers \
  src/main/scala/org/scalajs/dom/experimental/sharedworkers \
  src/main/scala/org/scalajs/dom/experimental/storage \
  src/main/scala/org/scalajs/dom/experimental/webgl \
  src/main/scala/org/scalajs/dom/experimental/webrtc \
  ; do
  l="$(cloc $f | tail -2 | head -1 | awk '{print $5}')"
  echo "$f  -- $l"
done

which yielded unexpected results:

src/main/scala/org/scalajs/dom/crypto  -- 569
src/main/scala/org/scalajs/dom/experimental/beacon  -- 20
src/main/scala/org/scalajs/dom/experimental/cachestorage  -- 40
src/main/scala/org/scalajs/dom/experimental/deviceorientation  -- 50
src/main/scala/org/scalajs/dom/experimental/domparser  -- 21
src/main/scala/org/scalajs/dom/experimental/gamepad  -- 43
src/main/scala/org/scalajs/dom/experimental/intl  -- 161
src/main/scala/org/scalajs/dom/experimental/mediastream  -- 376
src/main/scala/org/scalajs/dom/experimental/permissions  -- 60
src/main/scala/org/scalajs/dom/experimental/push  -- 81
src/main/scala/org/scalajs/dom/experimental/serviceworkers  -- 199
src/main/scala/org/scalajs/dom/experimental/sharedworkers  -- 28
src/main/scala/org/scalajs/dom/experimental/storage  -- 23
src/main/scala/org/scalajs/dom/experimental/webgl  -- 260
src/main/scala/org/scalajs/dom/experimental/webrtc  -- 361

Surprisingly the biggest packages seem to be:

569 - src/main/scala/org/scalajs/dom/crypto
376 - src/main/scala/org/scalajs/dom/experimental/mediastream
361 - src/main/scala/org/scalajs/dom/experimental/webrtc
260 - src/main/scala/org/scalajs/dom/experimental/webgl
199 - src/main/scala/org/scalajs/dom/experimental/serviceworkers
161 - src/main/scala/org/scalajs/dom/experimental/intl

So actually I think there are two good choices we could make:

  • packages with loc (or maybe type count) > n keep their packages?
  • just chuck everything in dom?

@sjrd @armanbilge: Thoughts?

@japgolly japgolly changed the title Move packages into dom Move more packages into dom Sep 3, 2021
@japgolly
Copy link
Contributor Author

japgolly commented Sep 3, 2021

I think my opinion here is to just put everything in dom. Like @sjrd said, the real dom API isn't namespaced so maybe we're just creating work for ourselves for no real benefit that I can think of atm.

@japgolly
Copy link
Contributor Author

japgolly commented Sep 3, 2021

Thinking towards an auto-generated future, this is also good. Since generators wouldn't know about our namespaces without manual interventions.

@armanbilge also had a good point in favour of no namespaces ^^

@armanbilge
Copy link
Member

just chuck everything in dom

My vote 👍

@armanbilge
Copy link
Member

Btw, can this happen at the same time as #476?

@sjrd
Copy link
Member

sjrd commented Sep 3, 2021

+1 for putting everything in dom. It seems simpler.

Btw, can this happen at the same time as #476?

I would advise against it. Moving without splitting can be done with almost zero impact on the git history, while it has an impact on the real code. Splitting without moving has no impact on the code, but destroys the git history. If you do both at the same time, you get a commit that is horrible to review and horrible to debug in a git blame situation.

@armanbilge
Copy link
Member

Splitting without moving has no impact on the code, but destroys the git history.

That's a really important point, especially if/while we are still merging things up from series/1.x into main.

@armanbilge armanbilge added this to the v2.0.0 milestone Sep 4, 2021
@armanbilge
Copy link
Member

@japgolly Ths is the last major binary-breaking change we have to make before we can release 2.0 right?

@japgolly
Copy link
Contributor Author

japgolly commented Sep 4, 2021

Nah there's this, there's 545, but I'm also planning on submitting a PR around IDB and maybe some other things but definitely hoping to get 2.0 soon! Hopefully within 2 weeks :)

@japgolly
Copy link
Contributor Author

japgolly commented Sep 4, 2021

oh lol this is #545 hahah

@japgolly
Copy link
Contributor Author

japgolly commented Sep 4, 2021

Oh also I want to go through the remaining issues and PR or close what I can too

@armanbilge
Copy link
Member

Cool! Always helpful to know your grand vision 🤩

I'll take another pass through the issues, I already did the easy ones unfortunately 😛 We could ask for community help too but I think they were a bit scared off by all the churn going on right now.

@armanbilge
Copy link
Member

I made a call for PRs on Discord/gitter, let's see if that gets us anything :)

Oh also I want to go through the remaining issues and PR or close what I can too

I think that's great, but also potentially a big time-suck 🙃 unless there's binary compatibility concerns about any of those, I think we shouldn't block 2.0 on as they can be added anytime. Like, opening an issue is cheap, and if someone wants something badly enough they can PR it. Rather than investing my time into random issues, personally I'd rather promise timely review/release for any contributed PRs.

@japgolly
Copy link
Contributor Author

japgolly commented Sep 5, 2021

I made a call for PRs on Discord/gitter, let's see if that gets us anything :)

Wonderful! ❤️

I think we shouldn't block 2.0

I've got a different view. We're the maintainers now, doing through the issues list and raising small PRs if applicable is just part of the job to me. Once the 2.0 release is done I expect our maintenance costs to go down to like 2% of what they are now: just review PRs, push tags and broadcast. There's no rush to get 2.0 out, although personally I'd like it out within the next two weeks. I'd rather take a little more time, do it properly, and hopefully not create any bincompat traps for ourselves (remember that once 2.0 is out, that's our bincomat baseline).

@japgolly
Copy link
Contributor Author

japgolly commented Sep 5, 2021

Oh btw, for the sake of clarity, if we have issues that would take too long to create and/or it's not important enough for anyone to bother right now, we should definitely leave that for a future 2.x release and not block 2.0.

@armanbilge
Copy link
Member

armanbilge commented Sep 5, 2021

We're the maintainers now, doing through the issues list and raising small PRs if applicable is just part of the job to me

Oh absolutely agree, I think the tricky part is what counts as "applicable" :) I mean #490 is bite sized (I could fix it instead of writing this) and like if at least a few people pile on saying they need that (and still nobody PRs! really!!), fair enough, but like otherwise so easy to be sucked in adding this and that because somebody made an issue but who knows if it will ever get used downstream. Also issues like that are perfect for someone who's new to OSS and wants to make their first PR etc.

I'd rather take a little more time, do it properly, and hopefully not create any bincompat traps for ourselves (remember that once 2.0 is out, that's our bincomat baseline

💯 to this.

@japgolly
Copy link
Contributor Author

japgolly commented Sep 5, 2021

I mean #490 is bite sized (I could fix it instead of writing this) and like if at least a few people pile on saying they need that (and still nobody PRs! really!!)

😆 😆 😆 Yeah welcome to OSS haha. Or at least in my experience it's always like that. Some people would rather spend 40min every day for weeks arguing for a change, rather than take 2 hours once to just PR it. I think it must just be some people's personality traits to be more inclined to discussion rather than doing (or something like that). It's all good though, or at least it is what it is. I've long accepted it :)

@japgolly
Copy link
Contributor Author

japgolly commented Sep 5, 2021

...which is to say, my usually OSS policy is I try to find time to just implement reasonably-sized changes that people request, but for big large features, unless I need it at the time too, I just announce that I can't justify working for free for so long on such a big change so pls PR or sponsor. To my great appreciation, scalajs-react has been sponsored twice for some big features and I'm really thankful for that cos it's a win for everyone. Once 2.0 is out, FYI, that's going to be my operating principal. I literally don't have a paying job or any income so it's just a fact of life for OSS that the amount of effort people put in for free (or in many cases at an opportunity cost), needs to be fair and voluntary. So don't worry, I won't be holding up 2.0 for too long or letting my inner perfectionist grab the reins 😀

@armanbilge
Copy link
Member

Once 2.0 is out, FYI, that's going to be my operating principal.

Sounds good! I mean, you do you :) I guess I'm still figuring out my OSS MO, but at least for this project (in its current manually-maintained state) I think my goal is more to be an enabler of do-ers (facades are really not that hard) rather than the do-er. And of the course the usual maintainer shebang of bugs, updates, etc. etc.

To my great appreciation, scalajs-react has been sponsored twice for some big features and I'm really thankful for that cos it's a win for everyone.

I wonder if we could set up sponsorship for scala-js-dom too, might be tricky since its technically an "official" project that is community-maintained. Sounds like you could use it 😉 (and quite frankly I wouldn't mind either!).

@japgolly
Copy link
Contributor Author

japgolly commented Sep 5, 2021

Yeah man, everyone brings something unique to the table; that to me is the beauty of collaboration. It sounds like we might be quite complimentary :)

I wonder if we could set up sponsorship for scala-js-dom too

I don't know. It would be nice but there'd have to be a specific goal and outcome IMO, so I'm not sure how qualified this project would be, all things considered. I don't know who'd sponsor either. Scala Centre is really rude and not very motivated. There are services like Patreon and Open Something Collective (?) that make it easy to fund OSS but in my experience I haven't got a great amount of support through those channels. The times I got sponsors a Scala user reached out to me on behalf of their organisation and we setup ad-hoc contracts. I'm not really a money (seeking) guy but if you have any ideas for some holistically symbiotic support then I'd love it and be very open to trying to work something out.

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.

3 participants