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.

Sign up for GitHub

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

Jump to bottom

chore(ecdsa): Replace nested if-match-if's with a single match #180

Open
wants to merge 8 commits into
base: mirroring
Choose a base branch
from
Open

chore(ecdsa): Replace nested if-match-if's with a single match #180

wants to merge 8 commits into from

Conversation

kpop-dfinity
Copy link
Contributor

Replaced

if {
  match  {
    None => if {} else {}
    Some => if {} else {} 
  }
} else {
  match  {
    None => if {} else {}
    Some => if {} else {} 
  }
},

which was hardly readable for me, with one big match

nmattia and others added 2 commits June 4, 2024 13:16
@nmattia
chore(IDX): remove usage of new_git_repository (#167)
cb05b91 
This replaces the last (`new_`)`git_repository` usage with the
recommended `http_archive`. This means that the client's Git is not used
anymore and all dependencies listed in WORKSPACE are fetched natively by
Bazel.
@kpop-dfinity
replace if-match-ifs with one big match
a81e239
@kpop-dfinity kpop-dfinity changed the base branch from master to mirroring June 4, 2024 15:02
@kpop-dfinity kpop-dfinity marked this pull request as ready for review June 4, 2024 15:03
@kpop-dfinity kpop-dfinity requested a review from a team as a code owner June 4, 2024 15:03
@sa-github-api sa-github-api requested a review from a team as a code owner June 4, 2024 15:05
@basvandijk
Copy link
Collaborator

@kpop-dfinity you want to make sure that your commit a81e239 is based of the mirroring branch instead of master. Otherwise your PR will show commits like cb05b91 which are already in master but not yet in mirroring.

cgundy and others added 2 commits June 5, 2024 05:45
@cgundy
chore(IDX): update slack notification (#181)
33efe5c 
It seems that the action "ready for review" triggers before "review
requested" so the payload doesn't always have the complete teams. This
fix changes the trigger and will both trigger if a PR is directly opened
as a non-draft and if additional reviewers are added later.
@basvandijk @nmattia
fix(IDX): do upload to the bazel-remote cache for systests (#179)
bd3f308 
Without this it's impossible to run system-tests locally because the
IC-OS images won't be uploaded to the bazel cache which means Farm / k8s
won't be able to download them to boot VMs.

---------

Co-authored-by: Nicolas Mattia <[email protected]>
@kpop-dfinity
Copy link
Contributor Author

@kpop-dfinity you want to make sure that your commit a81e239 is based of the mirroring branch instead of master. Otherwise your PR will show commits like cb05b91 which are already in master but not yet in mirroring.

hmmm, I thought I did, I must have done something wrong

@basvandijk
Copy link
Collaborator

One of the failures in the failing "CI Main / Bazel Build All Config Check (pull_request)" job will be fixed by #188. Not sure yet about the other failures though...

Sawchord
Sawchord approved these changes Jun 6, 2024
View reviewed changes
Copy link
Contributor

@Sawchord Sawchord left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@kpop-dfinity
Copy link
Contributor Author

One of the failures in the failing "CI Main / Bazel Build All Config Check (pull_request)" job will be fixed by #188. Not sure yet about the other failures though...

Great, thanks for looking into it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@basvandijk basvandijk basvandijk approved these changes

@Sawchord Sawchord Sawchord approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
6 participants
@kpop-dfinity @basvandijk @Sawchord @nmattia @cgundy @marko-k0