Skip to content

Clean tjs namespace, round 1#583

Merged
saghul merged 9 commits intomasterfrom
tjs-clean-ns
Jul 4, 2024
Merged

Clean tjs namespace, round 1#583
saghul merged 9 commits intomasterfrom
tjs-clean-ns

Conversation

@saghul
Copy link
Copy Markdown
Owner

@saghul saghul commented Jul 2, 2024

No description provided.

@saghul
core: don't expose getPid, getPpid and isWorker
497d416 
There are getters for the first 2, the latter is only meant to be used
internally.
saghul added 2 commits July 2, 2024 23:58
@saghul
core: gethostname() -> hostname getter
49c5851
@saghul
fs: refactor copyfile
afa744b 
- Rename to copyFile
- Drop the flags and implement the sensible behavior
saghul added 3 commits July 3, 2024 22:53
@saghul
fs: introduce tjs.remove replacing rm, rmdir and unlink
35e5e85
@saghul
core: rename some APIs to give them more JS friendly names
c70c231 
- exepath -> exePath
- mkdir -> makeDir
- homedir -> homeDir
- mkdtemp -> makeTempDir
- mkstemp -> makeTempFile
- readdir -> readDir
- realpath -> realPath
- tmpdir -> tmpDir
@saghul
core: simplify code for getters
ec86b35 
We can do it in C now.
@saghul saghul changed the title Clean tjs namespace Clean tjs namespace, round 1 Jul 3, 2024
@saghul saghul marked this pull request as ready for review July 3, 2024 21:37
saghul added 2 commits July 3, 2024 23:45
@saghul
core: make userInfo a property
1ec4802 
Also adjust the returned key names.
@saghul
core: make cwd a property
d76db02 
Also fix copying keys from core to the tjs global. We want to copy the
descriptors, just in case there is a property, like here, so we copy the
descriptor, not the value.
@saghul
Copy link
Copy Markdown
Owner Author

saghul commented Jul 3, 2024

@KaruroChori Please take a look! Thank you for making me think about this again. There was certaily a bunch of stuff that was easily actionable.

Now on to the next steps. I do think some namespacing can be done!

  • vm: serialize / deserialize / compile / evalBytecode / gc / versions? ( this one is pretty clear I think!)
  • hwInfo: availableParallelism, cpus, networkInterfaces

There are some items that seem to belong somewhat together, but I can't quite put my finger on it: platform / what's in uname / uptime / loadavg / memory-info?

Last, some other random thoughts:

  • PosixSocket is a low level API not to be used by most, so it probably belongs in some tjs:posix-socket module, that would move many constants too
  • Abstract getaddrinfo into some lookup() function a la Node

@saghul saghul mentioned this pull request Jul 4, 2024
Merged
@KaruroChori
Copy link
Copy Markdown
Contributor

KaruroChori commented Jul 4, 2024

That's great, I think this greatly improves the library. I will need a bit to check its details but overall it looks good.

There are some items that seem to belong somewhat together, but I can't quite put my finger on it: platform / what's in uname / uptime / loadavg / memory-info?

All this was stuffed in tjs.platform in my proposal, alongside the content of what you suggested to have in hwInfo. I consider them to be all platform-specific details so it did make sense for me to have them all together.

Your vm was basically my proposed engine, so I am very much supportive of this as well, and I like the presence of even more features in it compared to what I originally considered.

PosixSocket is a low level API not to be used by most, so it probably belongs in some tjs:posix-socket module, that would move many constants too
Abstract getaddrinfo into some lookup() function a la Node

I agree too

@saghul
Copy link
Copy Markdown
Owner Author

saghul commented Jul 4, 2024

That's great, I think this greatly improves the library. I will need a bit to check its details but overall it looks good.

Sweet. I'll likely merge this and continue with the next steps and refine as we go.

All this was stuffed in tjs.platform in my proposal, alongside the content of what you suggested to have in hwInfo. I consider them to be all platform-specific details so it did make sense for me to have them all together.

I think thay makes sense. The current tjs.platform is pretty much the sysname field of uname(2) so it can be extracted from there for sure.

KaruroChori
KaruroChori reviewed Jul 4, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@KaruroChori KaruroChori left a comment

Choose a reason for hiding this comment

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

Full review

@saghul saghul merged commit 1303987 into master Jul 4, 2024
@saghul saghul deleted the tjs-clean-ns branch July 4, 2024 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@KaruroChori KaruroChori KaruroChori left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@saghul @KaruroChori