Skip to content
This repository has been archived by the owner on Jan 26, 2021. It is now read-only.

Refactor #701

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Refactor #701

wants to merge 4 commits into from

Conversation

tejaswini22199
Copy link

Description

Include a summary of the change and relevant motivation/context. List any dependencies that are required for this change.
Refactored the code by removing multiple instances of payload and token objects
Fixes # [655]

Type of Change:

Delete irrelevant options.

  • Code

Code/Quality Assurance Only

How Has This Been Tested?

Describe the tests you ran to verify your changes. Provide instructions or GIFs so we can reproduce. List any relevant details for your test.

Checklist:

Delete irrelevant options.

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials

Code/Quality Assurance Only

  • My changes generate no new warnings

Copy link
Contributor

@SanketDG SanketDG left a comment

Choose a reason for hiding this comment

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

This works, but I was not looking to create globals variable here.

Instead I was expecting that one would want to separate out this code onto a function, so that there is one single source of truth for token

Also if you think, payload gets initialized when the file is loaded into memory (basically when the server starts), so this does not work because payload is time-dependent, so you need to initialize it when you call the methods create, edit etc.

@sakshi1499 sakshi1499 added the Status: Changes Requested Changes are required to be done by the PR author. label Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Status: Changes Requested Changes are required to be done by the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants