Skip to content

Conversation

guyp-descope
Copy link
Contributor

TODOs:

  1. Adjust some exceptions
  2. Add Docs (inline, Readme and Notion)
  3. Add the Code coverage badge (values are already calculated and stand on 93%)
  4. Remove some values used for the testing

@guyp-descope guyp-descope self-assigned this May 19, 2022
@aviadl
Copy link
Member

aviadl commented May 20, 2022

Didn't you review this with @dorsha ?

@dorsha
Copy link
Member

dorsha commented May 20, 2022

Didn't you review this with @dorsha ?

Not everything, there are still some leftovers

Copy link
Member

@dorsha dorsha left a comment

Choose a reason for hiding this comment

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

Very nice!

  1. See comments
  2. What is the empty pyproject.toml file?
  3. Don't forget to add the gitleaks
  4. Let me know once other stuff are ready so I will review again
  5. What is the part which is not covered in the coverage?

@dorsha
Copy link
Member

dorsha commented May 21, 2022

I see the coverage is only for the errors, if possible I would mock some of them to check the error behavior, since it is important in term of the sdk.

Copy link
Member

@aviadl aviadl left a comment

Choose a reason for hiding this comment

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

Who is writing all the py docs ?
What did we do with the go docs?

Need to sit with @jeff-descope and make prepare this process

@dorsha
Copy link
Member

dorsha commented May 23, 2022

Can we change it to be updated only in a single comment?

image

@guyp-descope
Copy link
Contributor Author

Can we change it to be updated only in a single comment?

image

I tried to do so and added the flag we talked about but looks like its didnt help..
I think Ill replace the badge component..

@dorsha
Copy link
Member

dorsha commented May 23, 2022

Yes, maybe better to replace with this, like we did in our services and in the console-app:
https://github.com/descope/console-app/blob/main/.github/workflows/ci.yml#L55

https://github.com/devmasx/coverage-check-action

@github-actions
Copy link

github-actions bot commented May 25, 2022

Coverage

Coverage Report
FileStmtsMissCover
TOTAL2540100%

Tests Skipped Failures Errors Time
19 0 💤 0 ❌ 0 🔥 0.748s ⏱️

…ve samples 4. started to add documentation (including README.md file)
Copy link
Member

@dorsha dorsha left a comment

Choose a reason for hiding this comment

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

Nice!

…he user. 2. add logic for all the refresh token 3. add more unittests 4. adjust the samples apps to the new logic and return values
Copy link
Member

@dorsha dorsha left a comment

Choose a reason for hiding this comment

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

Nice

@dorsha dorsha marked this pull request as ready for review May 30, 2022 08:17
@dorsha
Copy link
Member

dorsha commented May 30, 2022

So let's merge this one, and have the leftovers in a different PR

@guyp-descope guyp-descope merged commit a76d544 into main May 31, 2022
@guyp-descope guyp-descope deleted the draft branch May 31, 2022 07:20
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.

7 participants