Skip to content

Conversation

ppcano
Copy link
Collaborator

@ppcano ppcano commented Jun 19, 2023

No description provided.

@ppcano ppcano requested a review from olegbespalov June 19, 2023 08:37
Copy link
Contributor

@olegbespalov olegbespalov left a comment

Choose a reason for hiding this comment

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

How about explicitly naming GrpcExperimentalBlockquote to keep the context of what this blockquote is about? WDYT?

import Blockquote from 'components/shared/blockquote';
import React from 'react';

const GrpcBlockquote = () => (
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const GrpcBlockquote = () => (
const GrpcExperimentalBlockquote = () => (

</Blockquote>
);

export default GrpcBlockquote;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export default GrpcBlockquote;
export default GrpcExperimentalBlockquote;

Comment on lines +1 to +3
import GrpcBlockquote from './grpc-blockquote.view';

export default GrpcBlockquote;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import GrpcBlockquote from './grpc-blockquote.view';
export default GrpcBlockquote;
import GrpcExperimentalBlockquote from './grpc-blockquote.view';
export default GrpcExperimentalBlockquote;

import CryptoBlockquote from 'components/shared/crypto-blockquote';
import DescriptionList from 'components/shared/description-list';
import ExperimentalBlockquote from 'components/shared/experimental-blockquote';
import GrpcBlockquote from 'components/shared/grpc-blockquote';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import GrpcBlockquote from 'components/shared/grpc-blockquote';
import GrpcExperimentalBlockquote from 'components/shared/grpc-blockquote';

BlockingAwsBlockquote,
CryptoBlockquote,
WsBlockquote,
GrpcBlockquote,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
GrpcBlockquote,
GrpcExperimentalBlockquote,

---

The `k6/net/grpc` module, added in k6 v0.29.0, provides a [gRPC](https://grpc.io/) client for Remote Procedure Calls (RPC) over HTTP/2.
<GrpcBlockquote />
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<GrpcBlockquote />
<GrpcExperimentalBlockquote />

`Client` is a gRPC client that can interact with a gRPC server.

> ⚠️ **Note**: Only unary RPCs are currently supported, i.e. there is no support for client, server or bidirectional streaming.
<GrpcBlockquote />
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<GrpcBlockquote />
<GrpcExperimentalBlockquote />

@github-actions
Copy link
Contributor

There's a version of the docs published here:

https://mdr-ci.staging.k6.io/docs/refs/pull/1229/merge

It will be deleted automatically in 30 days.

@ppcano
Copy link
Collaborator Author

ppcano commented Jun 19, 2023

@olegbespalov,

There are other similar cases: cryptoBlockquote and wsBlockquote. For consistency, I'd have to change to cryptoExperimentalBlockquote and wsExperimentalBlockquote. Not a big issue.

But not sure because grpcExperimentalBlockquote could refer that the quote is for the experimental/grpc module, which is not the case.

atm, I'll leave it as it is. But let me know if you have a strong opinion about it.

@ppcano ppcano merged commit 1d99260 into main Jun 19, 2023
@ppcano ppcano deleted the grpc-cosmetics branch June 19, 2023 09:02
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.

2 participants