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

chore: code cleanup and re-org #154

Merged
merged 7 commits into from
Aug 10, 2023
Merged

chore: code cleanup and re-org #154

merged 7 commits into from
Aug 10, 2023

Conversation

gmaclennan
Copy link
Member

@gmaclennan gmaclennan commented Aug 9, 2023

  • Remove code replaced by new DataStore, DataType and IndexWriter
    implementations
  • Skip failing authStore tests for now - get tests running and passing again (pending updates to AuthStore for new Mapeo Schema)
  • Move all code to src folder to prepare for compiling typescript
    declaration files for package publishing
  • Update internal READMEs
  • Format everything with prettier
  • Add missing deps
  • Fix protobuf build script

Why move everything into src? For publishing we need to compile Typescript declarations (.d.ts). It's difficult to manage outputting these alongside the source files, it's easier to manage if all declarations go in a dist folder. However if there are JS files that need type-checked in the top-level project folder, then Typescript config can be a pain. Typescript is easiest to configure when all code that needs to be compiled is in a single folder, for which src makes the most sense.

- Remove code replaced by new DataStore, DataType and IndexWriter
  implementations
- Skip failing tests for now
- Move all code to `src` folder to prepare for compiling typescript
  declaration files for package publishing
@gmaclennan gmaclennan self-assigned this Aug 9, 2023
Copy link
Member

@achou11 achou11 left a comment

Choose a reason for hiding this comment

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

Couple of minor questions but otherwise lgtm!

Didn't want to comment on all potentially generated files, but would be helpful to have first-line comments for those files to indicate that they're generated, so we don't accidentally attempt to manually update them in the future

Copy link
Member

Choose a reason for hiding this comment

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

is this a generated file? if so, is there a way to add a comment here to indicate that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Not sure. Let's address in a separate issue (this PR doesn't change this file)

Copy link
Member

Choose a reason for hiding this comment

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

would test.todo be more appropriate for these or does it not really matter?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll just remember for now, will be working on this next week.

* main:
  chore: remove todo comments (#159)
@gmaclennan gmaclennan merged commit 3c34e71 into main Aug 10, 2023
6 checks passed
@achou11 achou11 deleted the chore/code-cleanup branch August 10, 2023 13:26
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