Skip to content

Merge corrected LuaLS annotation code#106

Merged
Derpius merged 6 commits intoTAServers:masterfrom
bjcscat:luals-annotations
Dec 2, 2023
Merged

Merge corrected LuaLS annotation code#106
Derpius merged 6 commits intoTAServers:masterfrom
bjcscat:luals-annotations

Conversation

@bjcscat
Copy link

@bjcscat bjcscat commented Nov 30, 2023

Ticket

Resolves requests made in #46

Changes

Changed the luals-annotations package to be in line with the changes requested.

Impact

These changes should allow for the luals annotations to be merged into master

Testing

Tests remain as they are in #46

@bjcscat bjcscat changed the title Luals annotations LuaLS annotations Nov 30, 2023
@bjcscat bjcscat changed the title LuaLS annotations merge corrected LuaLS annotation code Nov 30, 2023
@bjcscat bjcscat changed the title merge corrected LuaLS annotation code erge corrected LuaLS annotation code Nov 30, 2023
@bjcscat bjcscat changed the title erge corrected LuaLS annotation code Merge corrected LuaLS annotation code Nov 30, 2023
@Derpius
Copy link
Member

Derpius commented Nov 30, 2023

Thanks for this! I’ve had a quick scan on the way to work and looks good. I’ll do a proper review this weekend though (also planning to smash through the backlog a bit)

The commitlint pipeline is failing due to your commit not being prefixed, but im planning to replace conv commits with a changelog doc for the automated releases this weekend anyway

@bjcscat
Copy link
Author

bjcscat commented Nov 30, 2023

Did a force-push with simplified commit history and a proper prefix.

@bjcscat
Copy link
Author

bjcscat commented Nov 30, 2023

Additionally, the openresty CI job seems to be broken

Copy link
Member

@Derpius Derpius left a comment

Choose a reason for hiding this comment

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

I think this is good enough now (although I've left one nit on a typo). There's still a lot of tech debt from @yogwoggf's original implementation that would ideally be fixed, but given it just needs to generate annotations and doesn't affect any critical part of the codebase then I think merging now is the pragmatic solution

Taking a look at the openresty pipeline first, then I'll do a quick QA

@Derpius Derpius added the scope - docs Improvements or additions to documentation label Dec 2, 2023
@Derpius Derpius merged commit 2cca63b into TAServers:master Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope - docs Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments