Skip to content

Conversation

@KevLehman
Copy link
Member

@KevLehman KevLehman commented Aug 30, 2022

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

  • More tests
  • Remove try/catches from endpoint handlers
  • Remove Promise.await's from endpoint handlers
  • Remove unnecesary/duplicated permissions checks on endpoints

@lgtm-com
Copy link

lgtm-com bot commented Aug 30, 2022

This pull request introduces 2 alerts when merging 050186a into 473455a - view on LGTM.com

new alerts:

  • 2 for Syntax error

@codecov
Copy link

codecov bot commented Aug 30, 2022

Codecov Report

Merging #26749 (c88b04c) into develop (343c5f1) will increase coverage by 0.13%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #26749      +/-   ##
===========================================
+ Coverage    40.41%   40.54%   +0.13%     
===========================================
  Files          799      799              
  Lines        18289    18289              
  Branches      1959     1959              
===========================================
+ Hits          7392     7416      +24     
+ Misses       10601    10573      -28     
- Partials       296      300       +4     
Flag Coverage Δ
e2e 40.54% <ø> (+0.13%) ⬆️

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

@KevLehman KevLehman changed the title Chore: Make endpoints async Chore: Cleanup endpoint handlers Sep 5, 2022
@KevLehman KevLehman marked this pull request as ready for review September 5, 2022 19:44
@KevLehman KevLehman requested review from a team as code owners September 5, 2022 19:44
Copy link
Member

@debdutdeb debdutdeb left a comment

Choose a reason for hiding this comment

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

A quarter way One file through I'm moving on for today 🙈

@ggazzo ggazzo added stat: ready to merge PR tested and approved waiting for merge and removed stat: needs QA labels Sep 8, 2022
- removing unused variables
- permission checks which are no longer required
@kodiakhq kodiakhq bot merged commit 19be3b1 into develop Sep 9, 2022
@kodiakhq kodiakhq bot deleted the chore/make-endpoints-async-omn branch September 9, 2022 16:25
@tassoevan tassoevan mentioned this pull request Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants