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

Fixing the SAF keyring error through console logging it. #565

Open
wants to merge 2 commits into
base: v2.x/staging
Choose a base branch
from

Conversation

Atharva-Kanherkar
Copy link

Proposed changes

The SAF keyring error was an error which did not allow the server to start without an error, until we could find the fix, we have decided to bypass it and know the error through console logging,

Type of change

Please delete options that are not relevant.

  • Refactor the code

PR Checklist

  • If the changes in this PR are meant for the next release / mainline, this PR targets the "staging" branch.
  • My code follows the style guidelines of this project (see: Contributing guideline)
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

Copy link
Member

@1000TurquoisePogs 1000TurquoisePogs left a comment

Choose a reason for hiding this comment

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

Disabling an error to work-around a bug is not a solution to a bug.
Identify why the bug has happened and resolve that.

If this PR is based on an outdated copy of v2.x/staging, you may have missed that this was already fixed.

a7e5b99

There was a bug in which saf was changed from being initialized to undefined to instead be initialized to an empty array.

Previously if (saf) was done, which was previously false but then true when set to an empty array.

A fix was done to do if (saf.length > 0 instead.
This fix was made to v2.x/rc branch but not v2.x/staging branch.
Therefore the bug still existed in v2.x/staging branch.

But, the two branches were synchronized a week or two ago so you should check if the problem that you still have exists. You may not need a PR at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Change Requested
Development

Successfully merging this pull request may close these issues.

2 participants