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

feat: add support for the pesde package manager #26

Open
wants to merge 32 commits into
base: refactor/monorepository-origin
Choose a base branch
from

Conversation

4x8Matrix
Copy link
Member

@4x8Matrix 4x8Matrix commented Nov 28, 2024

This PR changes quite a bit, to go over the larger details:

  1. Remove the use of aliases
  2. Implement bundling + type support within the bundling for the Api Types package.
  3. Use Pesde's workspace feature
  4. Adopt Pesde as Discord-Luau's package manager
  5. Implement an entrypoint into each package.
  6. Implement a script framework, just to have a clear distinction between whats just a temporary get stuff done quickly script, vs's a genuine script, such as one used in the Ci
  7. Migrate the /test folder into their respective packages.
  8. Remove the Vendor folder
  9. Refactor future library -> async library.

Quirks, things I don't like atm:

  • buffer isn't really a buffer, buffer is more like a string stream, should this be renamed?
  • core is an odd name - should we call this something else? or is this fine..

@4x8Matrix 4x8Matrix requested a review from CompeyDev November 28, 2024 21:05
@4x8Matrix
Copy link
Member Author

also I know I just made a +3902, -2789 change within a single commit @CompeyDev - please forgive as I have sinned.

I'll be adopting versioned code soon after this as otherwise i'm going to be causing myself crazy headaches trying to do things on the fly..

@4x8Matrix
Copy link
Member Author

CI is now failing with error: library file at /home/runner/work/discord-luau/discord-luau/packages/websocket/lune_packages/.pesde/discord_luau+api_types/0.1.0/api_types/bundled.luau not found

pls also review the pipeline, I'm guessing that what we're currently doing is shambles because we're using rokit, that reads from aftman.toml.. but we want to drop all of that anyway to use pesde x? I shall wait until your review to further expand on this.

Copy link
Member

@CompeyDev CompeyDev left a comment

Choose a reason for hiding this comment

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

Some other nitpicks / misc changes I would suggest:

  • Using a proper async primitive as discussed on Discord; Future along with a yielding variant of the package makes the most sense
  • Add a pinned pesde_version field to all manifests, since pesde will have breaking changes and we want to prevent confusion among contributors; see pesde docs
  • Fix missing manifest metadata for some packages
  • Get rid of type patching hack once Support Luau types when bundling seaofvoices/darklua#144 is implemented
  • Symlink top-level license into packages, mark the license field, and include a copy of the symlinked license in the includes field of all packages to comply with MIT
  • Draft proper READMEs for individual packages
  • Rename packages, they are currently ambiguous (discordluau/classes is a prime example, what are they classes of? it is hard for a spectator to tell)
    • there's also discordluau/core which is confusing as raised by the initial comment
  • For an initial publish, we should probably start versioning from v0.0.1 for some packages (mostly all the non-util ones)
  • Can we call all the requires aliased as utilities as util instead? It is currently unnecessarily verbose
  • Should improve wording of package descriptions
  • Noticed a bundled.luau exception in the top-level .gitignore, and from what I see, only the api-types package contains this, so it would make sense to have this particular exclusion in packages/api-types/.gitignore
    bundled.luau

aftman.toml Outdated Show resolved Hide resolved
packages/api-types/pesde.toml Outdated Show resolved Hide resolved
packages/api-types/pesde.toml Show resolved Hide resolved
packages/api-types/src/application.luau Outdated Show resolved Hide resolved
packages/api-types/pesde.toml Outdated Show resolved Hide resolved
pesde.toml Show resolved Hide resolved
packages/buffer/pesde.toml Outdated Show resolved Hide resolved
packages/bit/pesde.toml Outdated Show resolved Hide resolved
packages/voice/pesde.toml Outdated Show resolved Hide resolved
packages/websocket/pesde.toml Outdated Show resolved Hide resolved
@CompeyDev
Copy link
Member

pls also review the pipeline, I'm guessing that what we're currently doing is shambles because we're using rokit, that reads from aftman.toml.. but we want to drop all of that anyway to use pesde x? I shall wait until your review to further expand on this.

pesde/tooling is nearly stable and complete, and I'll work towards creating releases for most common tooling, we can then migrate to dev dependencies and merge this.

@CompeyDev
Copy link
Member

The CI install step fails because the api-types package manifest contains this:

[target]
environment = "lune"
lib = "bundled.luau"

Which references a bundled.luau file which does not exist on the CI container until some script is executed. In this sort of situation a preinstall script is useful, but that's not a feature in pesde yet. For now, we can settle for manually generating the bundled types before running install.

@CompeyDev
Copy link
Member

@4x8Matrix You have missed adding the pesde_version field to some manifests, I currently only see it for the api-typespackage. Please fix.

@4x8Matrix
Copy link
Member Author

@CompeyDev lol I was in the api-types package when I tried to commit everything - git doesn't add things outside of the current directory - which meant only the above was commited :(

fixed

@CompeyDev
Copy link
Member

lol I was in the api-types package when I tried to commit everything - git doesn't add things outside of the current directory - which meant only the above was commited :(

I think you're using git wrong. git add . will only stage files from the current directory. To stage all files you will need to run git add -A.

@4x8Matrix
Copy link
Member Author

oh, thats cool

@4x8Matrix 4x8Matrix requested a review from CompeyDev December 14, 2024 13:47
+ updated the README for each individual package.
+ updated the pinned version of each package to the latest pesde release
+ Drafted proper README that help indicate the purpose of each package
+ Replace discordluau/core with discordluau/discordluau
+ Updated versions to be 0.0.1 instead of 0.1.0
+ Updated Utilities -> Util
+ Improved wording on READMEs
+ Updated .gitignore for the bundled.luau file

! Implemented Futures over Async, we're now using Futures!

I am yet to test these changes, we're still waiting on seaofvoices/darklua#144 until we can genuinely test this.
@4x8Matrix
Copy link
Member Author

Whoops, forgot to update some calls, unticking the future impl

.github/workflows/update-documentation.yml Outdated Show resolved Hide resolved
.gitignore Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
.lune/script.luau Outdated Show resolved Hide resolved
packages/api-types/README.md Outdated Show resolved Hide resolved
packages/buffer/README.md Outdated Show resolved Hide resolved
packages/builders/src/init.luau Outdated Show resolved Hide resolved
packages/classes/README.md Outdated Show resolved Hide resolved
@4x8Matrix 4x8Matrix requested a review from CompeyDev January 14, 2025 22:36
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.

2 participants