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

feat!: rename function to archive #393

Merged
merged 6 commits into from
Feb 8, 2024
Merged

Conversation

SdgJlbl
Copy link
Contributor

@SdgJlbl SdgJlbl commented Oct 13, 2023

@SdgJlbl SdgJlbl changed the title feat: rename function to archive feat!: rename function to archive Oct 18, 2023
@SdgJlbl SdgJlbl force-pushed the feat/rename-function-to-archive branch from a9673b4 to ef92a1d Compare October 18, 2023 14:29
@SdgJlbl
Copy link
Contributor Author

SdgJlbl commented Oct 18, 2023

/e2e help

@Owlfred
Copy link

Owlfred commented Oct 18, 2023

Usage: /e2e [options] [help]

/e2e may appear anywhere as long as it is on its own line

Options:
  --refs <value>                                   Extra refs (branch or tag) with format REPO=GIT_REF,REPO=GIT_REF.
  Supported repositories: hlf-k8s, orchestrator, substra-backend, substra-frontend, substra-tools, substrafl, substra, substra-tests, substra-documentation, substra-ci.
  Example: /e2e --refs substra-backend=some_branch,orchestrator=some_tag (default: {})
  --tests-to-run, --tests <tests-to-run>           Comma-separated list of tests to run. Valid options: sdk, substrafl, doc, frontend, mnist, camelyon or NONE. (default: "sdk")
  --orchestrator-mode, --mode <orchestrator-mode>  Comma-separated list of orchestrator modes to run tests for. Valid options: standalone,distributed (default: "standalone,distributed")
  -h, --help                                       display help for command

@SdgJlbl SdgJlbl marked this pull request as ready for review October 18, 2023 14:33
@SdgJlbl SdgJlbl requested a review from a team as a code owner October 18, 2023 14:33
Copy link
Member

@ThibaultFy ThibaultFy left a comment

Choose a reason for hiding this comment

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

Thanks :)
But there is a mention to it in SubstraFL. You need to open a companion PR there too:
https://github.com/Substra/substrafl/blob/3f4434a11288ab416afaf33a5800cc2c120d83f0/tests/remote/register/test_register.py#L74

This is the only mention I found so it might just be a one line PR :) (I looked in substra tests, subtrafl and substra-doc)

@SdgJlbl
Copy link
Contributor Author

SdgJlbl commented Oct 18, 2023

Thanks :) But there is a mention to it in SubstraFL. You need to open a companion PR there too: https://github.com/Substra/substrafl/blob/3f4434a11288ab416afaf33a5800cc2c120d83f0/tests/remote/register/test_register.py#L74

This is the only mention I found so it might just be a one line PR :) (I looked in substra tests, subtrafl and substra-doc)

Nice catch 👍

I'm wondering if I might need to change some things in substra-tests too, in the factory possibly... 😭

@ThibaultFy
Copy link
Member

Thanks :) But there is a mention to it in SubstraFL. You need to open a companion PR there too: https://github.com/Substra/substrafl/blob/3f4434a11288ab416afaf33a5800cc2c120d83f0/tests/remote/register/test_register.py#L74
This is the only mention I found so it might just be a one line PR :) (I looked in substra tests, subtrafl and substra-doc)

Nice catch 👍

I'm wondering if I might need to change some things in substra-tests too, in the factory possibly... 😭

I looked and didn't find any :)

Copy link
Member

@ThibaultFy ThibaultFy left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. To be merged with the POC functions then ? (which mean we'll need to release substra too ?)

@SdgJlbl
Copy link
Contributor Author

SdgJlbl commented Oct 18, 2023

Thanks for the PR. To be merged with the POC functions then ? (which mean we'll need to release substra too ?)

Not with the POC branch, it's the next bunch of PRs (adding checksums, permission checks and a bit of refactoring / renaming) 😅

I anticipate lots of fun to rebase all that 😂

Copy link
Contributor

@guilhem-barthes guilhem-barthes left a comment

Choose a reason for hiding this comment

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

LGTM

@ThibaultFy ThibaultFy force-pushed the feat/rename-function-to-archive branch from 1dce7e7 to 6e6a9da Compare January 11, 2024 15:49
@ThibaultFy ThibaultFy force-pushed the feat/rename-function-to-archive branch from 6e6a9da to ce12f8c Compare January 11, 2024 15:53
SdgJlbl and others added 4 commits January 11, 2024 16:57
@ThibaultFy ThibaultFy force-pushed the feat/rename-function-to-archive branch from 2cd4f9e to 307bb0c Compare February 1, 2024 15:25
Signed-off-by: ThibaultFy <[email protected]>
@ThibaultFy ThibaultFy force-pushed the feat/rename-function-to-archive branch from 307bb0c to 238e116 Compare February 1, 2024 15:29
@ThibaultFy ThibaultFy merged commit 5fdc23f into main Feb 8, 2024
6 checks passed
@ThibaultFy ThibaultFy deleted the feat/rename-function-to-archive branch February 8, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants