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

Add --key-config-path alias #188

Merged
merged 4 commits into from
Jul 25, 2023
Merged

Add --key-config-path alias #188

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c0dadfb
Add alias.
bgravenorst Jul 12, 2023
51b2354
Update per Consensys rebranding.
bgravenorst Jul 12, 2023
1cbbd5b
Make key-config-path the preffered option.
bgravenorst Jul 23, 2023
d01eef7
Merge branch 'main' into WEB-185
bgravenorst Jul 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/Reference/CLI/options.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ data-path: "/Users/me/my_node/data"

Directory in which to store temporary files.

### `key-store-path`
### `key-store-path`, `key-config-path`
Copy link
Contributor

Choose a reason for hiding this comment

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

key-config-path is the preferred name so maybe this should come first?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure updating the title is the right thing either. Compare to other options with an alias... https://doc-web3signer-git-fork-bgravenorst-web-185-infura-web.vercel.app/Reference/CLI/options#logging

Copy link
Contributor Author

@bgravenorst bgravenorst Jul 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'll make key-config-path first (I just copied how it is in the CLI output). We do the aliases for --logging | -l, --version | -V, and '--help | -h` differently due to the single dash (-) used by the alias.

But generally we update the title to display the aliases. For example, in Teku.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On a seperate note, shouldn't we also create an alias for --key-store-config-file-max-size?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @bgravenorst that makes sense. Yes, maybe we should create that alias too, good spot, I'll bring it up with the team


<!--tabs-->

Expand Down
4 changes: 2 additions & 2 deletions docs/Reference/security-disclosure.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
---
description: Web3Signer security disclosure policy statement
description: Web3signer security disclosure policy statement
sidebar_position: 4
---

# Security disclosure policy

At ConsenSys, security is a priority. But regardless of how much effort we put into system security, there may still be vulnerabilities present. If you discover a vulnerability, we want to know about it so we can take steps to address it as quickly as possible. You can help us better protect our clients and our systems.
At Consensys, security is a priority. But regardless of how much effort we put into system security, there may still be vulnerabilities present. If you discover a vulnerability, we want to know about it so we can take steps to address it as quickly as possible. You can help us better protect our clients and our systems.

Please do the following:

Expand Down
16 changes: 8 additions & 8 deletions docusaurus.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const redocusaurus = [

/** @type {import('@docusaurus/types').Config} */
const config = {
title: "ConsenSys Web3Signer",
title: "Consensys Web3Signer",
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure which way is correct but maybe this should be Web3signer too

Copy link
Contributor Author

@bgravenorst bgravenorst Jul 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe we should do Web3Signer -> Web3signer in a separate PR? We'll need to ensure the codebase gets updated as well, see: https://github.com/Consensys/web3signer/blob/master/CONTRIBUTING.md

Let me know what you think. I'll gladly fix it up throughout this document if you feel I should go ahead with the changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know which is better/more correct. If there's a requirement to change it though then agree separate PR is best

Copy link
Contributor Author

@bgravenorst bgravenorst Jul 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Will confirm which is the preferred name, and update it (if required) in a seperate PR. Could you please approve the PR @siladu?

Copy link
Contributor

Choose a reason for hiding this comment

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

@bgravenorst just submitted more comments

tagline:
"An open-source, client-agnostic, Ethereum signing service written in Java that is capable of signing on multiple platforms..",
url: "https://docs.web3signer.consensys.net",
Expand All @@ -52,7 +52,7 @@ const config = {

// GitHub pages deployment config.
// If you aren't using GitHub pages, you don't need these.
organizationName: "ConsenSys", // Usually your GitHub org/user name.
organizationName: "Consensys", // Usually your GitHub org/user name.
projectName: "doc.web3signer", // Usually your repo name.
deploymentBranch: "gh-pages", // Github Pages deploying branch

Expand All @@ -71,7 +71,7 @@ const config = {
docs: {
sidebarPath: require.resolve("./sidebars.js"),
// Set a base path separate from default /docs
editUrl: "https://github.com/ConsenSys/doc.web3signer/tree/main/",
editUrl: "https://github.com/Consensys/doc.web3signer/tree/main/",
path: "docs",
routeBasePath: "/",
// @ts-ignore
Expand Down Expand Up @@ -155,7 +155,7 @@ const config = {
},
items: [
{
href: "https://github.com/ConsenSys/web3signer",
href: "https://github.com/Consensys/web3signer",
className: "header-github-link",
position: "right",
},
Expand Down Expand Up @@ -215,21 +215,21 @@ const config = {
title: "Community",
items: [
{
label: "ConsenSys Discord",
label: "Consensys Discord",
href: "https://discord.com/invite/consensys",
},
{
label: "Web3Signer GitHub",
href: "https://github.com/ConsenSys/web3signer",
href: "https://github.com/Consensys/web3signer",
},
{
label: "Web3Signer documentation GitHub",
href: "https://github.com/ConsenSys/doc.web3signer",
href: "https://github.com/Consensys/doc.web3signer",
},
],
},
],
copyright: `© ${new Date().getFullYear()} ConsenSys, Inc.`,
copyright: `© ${new Date().getFullYear()} Consensys, Inc.`,
},
prism: {
theme: lightCodeTheme,
Expand Down