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

Glitch 1 to implement 1 #31

Merged
merged 11 commits into from
May 20, 2020
Merged

Glitch 1 to implement 1 #31

merged 11 commits into from
May 20, 2020

Conversation

schettn
Copy link
Member

@schettn schettn commented May 18, 2020

Checklist:

  • Have you added an explanation of what your changes do and why you'd like them to be included?
  • Have you updated or added documentation for the change?
  • Have you tested your changes with successful results?

Type of Changes:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

What is the current behavior? (link to any open issues here)

What is the new behavior (if this is a feature change)?

Other information:
As written in the coding guidelines snek-tonic, no more points are set at JSDoc headers. Exceptions include cases in which a comment spans several lines.

Over time, the intel should be adapted to the new coding guide line. #32 #33

Now it is possible to fetch DOM pages again. Because of the invalid header which was set, querying DOM was not possible.
The start of the session always returns the same object structure.
Cookie-utils has been replaced with js-cookie.
The alive checks and the refresh method have been updated.
Now a Apollo response can contain errors.
If a specific authentication related error occurs, steps are taken to maintain the session.
Documentation has been added to the new code parts.
The start of the session always returns the same object structure now. Even though the interfaces had been correctly adjusted before, the structure of the return value was not constant.
The file which contains the task error has been moved to a other location.
@schettn schettn added bug Something isn't working enhancement New feature or request invalid This doesn't seem right labels May 18, 2020
@schettn schettn self-assigned this May 18, 2020
Copy link
Member

@Aichnerc Aichnerc left a comment

Choose a reason for hiding this comment

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

Looks clean ⚽

src/session/index.ts Outdated Show resolved Hide resolved
src/session/sessions.ts Outdated Show resolved Hide resolved
src/session/sessions.ts Outdated Show resolved Hide resolved
Copy link
Member

@pinterid pinterid left a comment

Choose a reason for hiding this comment

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

Looks good after some changes.

@schettn schettn requested a review from pinterid May 18, 2020 16:20
src/session/sessions.ts Outdated Show resolved Hide resolved
Copy link
Member

@pinterid pinterid left a comment

Choose a reason for hiding this comment

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

Looks good after a change.

The code quality has been improved due to a request of @pinterid.
Copy link
Member

@kleberbaum kleberbaum left a comment

Choose a reason for hiding this comment

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

Should be ok to merge after a few changes.

src/session/index.ts Outdated Show resolved Hide resolved
src/session/sessions.ts Outdated Show resolved Hide resolved
Copy link
Member

@pinterid pinterid left a comment

Choose a reason for hiding this comment

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

Looks good.

The code quality has been improved due to a request of @kleberbaum.
@schettn schettn requested a review from kleberbaum May 19, 2020 14:29
Copy link
Member

@kleberbaum kleberbaum left a comment

Choose a reason for hiding this comment

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

Should be ok to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request invalid This doesn't seem right
Projects
None yet
4 participants