Skip to content

Issue 1327 additional ekm updates - #1382

Merged
IvanPizhenko merged 14 commits into
masterfrom
issue_1327_additional_ekm_updates
Aug 6, 2021
Merged

Issue 1327 additional ekm updates#1382
IvanPizhenko merged 14 commits into
masterfrom
issue_1327_additional_ekm_updates

Conversation

@DenBond7

Copy link
Copy Markdown
Collaborator

This PR added additional ekm updates

close#1327 // if this PR closes an issue


Tests(delete all except exactly one):

  • Tests added or updated

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities

@DenBond7DenBond7 added the PR submitted PR is submitted for this issue label Aug 4, 2021
@DenBond7DenBond7 added this to the 1.2.0: More OrgRules milestone Aug 4, 2021
@DenBond7
DenBond7 marked this pull request as ready for review August 5, 2021 08:02
@IvanPizhenko

Copy link
Copy Markdown
Contributor

@DenBond7 why do you add label "PR Submitted" to pull requests? I have introduced this label with intent to put it on issues to help myself to see what issues in the list of issues assigned to me are already done (but not merged & closed yet), and what are not and so can be potentially be the next thing to work on.

return getUserFromBaseSettings("no_backups@flowcrypt.test")
}

fun getUserWithOrgRules(orgRules: OrgRules): AccountEntity {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suggest to rename this into getTestUserWithOrgRules.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

I'm not sure. This class is located in the tests package. So all things there relate to testing. And adding Test just will increase the method name.

If you don't have any new questions or recommendations please approve the current PR. It is blocking me a little to complete another one.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok, have not noticed it is in the test package

@DenBond7

Copy link
Copy Markdown
CollaboratorAuthor

@DenBond7 why do you add label "PR Submitted" to pull requests? I have introduced this label with intent to put it on issues to help myself to see what issues in the list of issues assigned to me are already done (but not merged & closed yet), and what are not and so can be potentially be the next thing to work on.

Ah... Sorry, I didn't know. I thought you have created it to mark pull requests. No prob. From now, I will not use it.

@IvanPizhenko
IvanPizhenko merged commit 4f0a535 into masterAug 6, 2021
@IvanPizhenko
IvanPizhenko deleted the issue_1327_additional_ekm_updates branch August 6, 2021 06:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR submittedPR is submitted for this issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

additional EKM updates to the app

2 participants

@DenBond7@IvanPizhenko