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.

Sign up for GitHub

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

Jump to bottom

Rndlabs chore repo prep #136

Merged
merged 7 commits into from
Aug 4, 2023
Merged

Rndlabs chore repo prep #136

merged 7 commits into from
Aug 4, 2023

Conversation

anxolin
Copy link
Contributor

@anxolin anxolin commented Aug 3, 2023 •
edited
Loading

Re-created the PR: #135

This PR:

  1. Provides comprehensive typedoc annotations to all exported functions.
  2. Extracts constants used across sub-modules to the consts.ts.
  3. Removes dead code (such as transformError.ts).
  4. Standardises variable nomenclature (ie. orderUid ✅, orderId 🙅‍♂️)
  5. Modifies the Github CI/CD such that the coveralls tasks only execute if done so in the context of a repository that is configured for such (makes it nicer for those who fork to run Github actions).
  6. Use a specific commit for the openapi.yml such that the package builds are always reproducible.

@anxolin anxolin marked this pull request as ready for review August 3, 2023 13:27
@coveralls
Copy link
Collaborator

coveralls commented Aug 3, 2023 •
edited
Loading

Coverage Status

coverage: 72.93% (+1.4%) from 71.565% when pulling 32f140f on rndlabs-chore-repo-prep into ad0c108 on main.

anxolin
anxolin commented Aug 3, 2023
View reviewed changes
package.json Show resolved Hide resolved
anxolin
anxolin commented Aug 3, 2023
View reviewed changes
Copy link
Contributor Author

@anxolin anxolin left a comment

Choose a reason for hiding this comment

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

APPROVED @mfw78 extremely good PR. Thanks a lot for this!

src/order-book/api.ts Show resolved Hide resolved
src/order-book/api.ts Show resolved Hide resolved
src/order-book/api.ts Show resolved Hide resolved
src/order-book/api.ts Show resolved Hide resolved
@anxolin anxolin requested a review from a team August 3, 2023 13:42
@anxolin
Copy link
Contributor Author

anxolin commented Aug 3, 2023

Only concern before merging, i'd like to clarify this #136 (comment)

alfetopito
alfetopito approved these changes Aug 3, 2023
View reviewed changes
Copy link
Contributor

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

One nitpick.
Otherwise, thanks so much for documenting it ❤️

package.json Outdated Show resolved Hide resolved
mfw78
mfw78 approved these changes Aug 4, 2023
View reviewed changes
package.json Show resolved Hide resolved
src/order-book/api.ts Show resolved Hide resolved
anxolin
anxolin commented Aug 4, 2023
View reviewed changes
Copy link
Contributor Author

@anxolin anxolin left a comment

Choose a reason for hiding this comment

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

APPROVED

@mfw78 mfw78 merged commit 3abfdcf into main Aug 4, 2023
6 checks passed
@mfw78 mfw78 deleted the rndlabs-chore-repo-prep branch August 4, 2023 12:13
@github-actions github-actions bot locked and limited conversation to collaborators Aug 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alfetopito alfetopito alfetopito approved these changes

@mfw78 mfw78 mfw78 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@anxolin @coveralls @alfetopito @mfw78