-
Notifications
You must be signed in to change notification settings - Fork 0
Added test pipeline and lint #2
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
base: development/1.0
Are you sure you want to change the base?
Conversation
Hello sylvainsenechal,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
.github/workflows/test.yml
Outdated
| - name: Wait for metadata to be ready | ||
| run: | | ||
| set -o pipefail | ||
| bash .github/workflows/wait_for_local_port.bash 9000 40 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also stolen from cloudserver, need it to run metadata backend
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
7fa7e8b to
96e82f2
Compare
c63bfee to
22f1240
Compare
.eslintrc.js
Outdated
| @@ -0,0 +1,26 @@ | |||
| module.exports = { | |||
| parser: '@typescript-eslint/parser', | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have a scality one too ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not too sure about this, on other codebases i see an eslint.config.mjs file that's more complex, don't know if I should use something similar or not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer yours to be honest but. i think we can extends also the one from scality. It's just an extends, so should be fine ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i extended the one from scality
17dca6a to
b424f76
Compare
|
|
||
| cloudserver-metadata: | ||
| image: ghcr.io/scality/cloudserver:9.1.4 | ||
| platform: linux/amd64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this really needed ? ( it's mac specific I think )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah, so it's indeed useless for the ci, but for mac dev who wants to start the docker-compose locally, it will be useful so that's why I added it
.eslintrc.js
Outdated
| '@typescript-eslint/no-explicit-any': 'warn', | ||
| '@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_' }], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure we only want these two rules ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually im gonna extend the scality linter, hold on
| - MONGODB_HOSTS=mongodb:27018 | ||
| - MONGODB_RS=rs0 | ||
| - REMOTE_MANAGEMENT_DISABLE=true | ||
| - SCALITY_ACCESS_KEY_ID=accessKey1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure it's very safe to put this here in general => should be a secret instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed, actually not needed at all since they are used by default when not provided
ISSUE: CLDSRVCLT-2
ISSUE: CLDSRVCLT-2
de57b47 to
0b8ba1f
Compare
8dd3788 to
58ae441
Compare
58ae441 to
dbfab6d
Compare
Issue: CLDSRVCLT-2