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

refactor: Remove ImportData meteor model #28458

Merged
merged 11 commits into from Mar 20, 2023
Merged

Conversation

KevLehman
Copy link
Contributor

@KevLehman KevLehman commented Mar 16, 2023

ARCH-918

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

Merging #28458 (6bf7bcc) into develop (c9bbad7) will increase coverage by 0.26%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #28458      +/-   ##
===========================================
+ Coverage    32.74%   33.00%   +0.26%     
===========================================
  Files          620      634      +14     
  Lines        12273    12579     +306     
  Branches      1865     1853      -12     
===========================================
+ Hits          4019     4152     +133     
- Misses        8052     8215     +163     
- Partials       202      212      +10     
Flag Coverage Δ
e2e 33.00% <ø> (+0.26%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@KevLehman KevLehman marked this pull request as ready for review March 18, 2023 00:15
Base automatically changed from refactor/models to develop March 20, 2023 15:09
@kodiakhq kodiakhq bot requested review from a team as code owners March 20, 2023 15:09
@KevLehman KevLehman changed the title refactor: Remove ImportData meteor model refactor: Remove ImportData meteor model Mar 20, 2023
apps/meteor/server/lib/ldap/Connection.ts Outdated Show resolved Hide resolved
apps/meteor/server/lib/ldap/Connection.ts Outdated Show resolved Hide resolved
apps/meteor/server/lib/ldap/Connection.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@pierre-lehnen-rc pierre-lehnen-rc left a comment

Choose a reason for hiding this comment

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

Actually, can you leave the whole Connection.ts file untouched (adding Promise.awaits elsewhere)?

Instead of reworking the ldap to work asynchronously, I'll split the Virtual Importer from the Base code so that the BaseImporter will be async and Virtual continues to be sync.

@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: needs QA labels Mar 20, 2023
@dionisio-bot dionisio-bot bot removed the stat: ready to merge PR tested and approved waiting for merge label Mar 20, 2023
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Mar 20, 2023
@kodiakhq kodiakhq bot merged commit 6d17b2b into develop Mar 20, 2023
36 checks passed
@kodiakhq kodiakhq bot deleted the refactor/import-data-model branch March 20, 2023 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat: QA skipped stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants