You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/05/23 14:32:19 UTC
[GitHub] [beam] damccorm opened a new pull request, #17732: Clean up uses of == instead of === in ts sdk
damccorm opened a new pull request, #17732:
URL: https://github.com/apache/beam/pull/17732
This updates to prefer === instead of == per https://google.github.io/styleguide/tsguide.html#equality-checks (and our readme)
R: @robertwb
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [x] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [x] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] robertwb commented on a diff in pull request #17732: Clean up uses of == instead of === in ts sdk
Posted by GitBox <gi...@apache.org>.
robertwb commented on code in PR #17732:
URL: https://github.com/apache/beam/pull/17732#discussion_r886083593
##########
sdks/typescript/src/apache_beam/worker/pardo_context.ts:
##########
@@ -65,21 +63,23 @@ export class ParamProviderImpl implements ParamProvider {
// if they are widely shared.
augmentContext(context: any) {
this.prefetchCallbacks = [];
- if (typeof context != "object") {
+ if (typeof context !== "object") {
return context;
}
const result = Object.create(context);
for (const [name, value] of Object.entries(context)) {
// Is this the best way to check post serialization?
if (
- typeof value == "object" &&
- value != null &&
- value["parDoParamName"] != undefined
+ typeof value === "object" &&
+ value !== null &&
+ value !== undefined &&
Review Comment:
This will never be the case if value === object.
##########
sdks/typescript/src/apache_beam/worker/worker.ts:
##########
@@ -194,7 +194,7 @@ export class Worker {
this.bundleProcessors.set(descriptorId, []);
}
const processor = this.bundleProcessors.get(descriptorId)?.pop();
- if (processor != undefined) {
+ if (processor !== null && processor !== undefined) {
Review Comment:
Let's change this to `if (processor)`.
##########
sdks/typescript/src/apache_beam/worker/worker.ts:
##########
@@ -321,8 +321,8 @@ export class BundleProcessor {
}
getStateProvider() {
- if (this.stateProvider == undefined) {
- if (typeof this.getStateChannel == "function") {
+ if (this.stateProvider === null || this.stateProvider === undefined) {
Review Comment:
same
##########
sdks/typescript/src/apache_beam/runners/portable_runner/runner.ts:
##########
@@ -89,7 +89,11 @@ class PortableRunnerPipelineResult implements PipelineResult {
let pollMillis = 10;
while (!PortableRunnerPipelineResult.isTerminal(state)) {
const now = Date.now();
- if (duration !== undefined && now - start > duration) {
+ if (
+ duration !== null &&
Review Comment:
I don't think we need to allow null here.
##########
sdks/typescript/src/apache_beam/runners/direct_runner.ts:
##########
@@ -304,8 +304,9 @@ function rewriteSideInputs(p: runnerApi.Pipeline, pipelineStateRef: string) {
const transforms = p.components!.transforms;
for (const [transformId, transform] of Object.entries(transforms)) {
if (
- transform.spec != undefined &&
- transform.spec.urn == parDo.urn &&
+ transform.spec !== null &&
Review Comment:
This should probably be `transform?.spec.urn` rather than an explicit check.
##########
sdks/typescript/src/apache_beam/transforms/sql.ts:
##########
@@ -49,7 +49,7 @@ export function sqlTransform<
// TOOD: (API) (Typescript): How to infer input_types, or at least make it optional.
async function expandInternal(input: InputT): Promise<PCollection<any>> {
function withCoder<T>(pcoll: PCollection<T>, type): PCollection<T> {
- if (type == null) {
+ if (type === null || type === undefined) {
Review Comment:
I think we can safely write this `if (type)`
##########
sdks/typescript/src/apache_beam/runners/runner.ts:
##########
@@ -27,15 +27,19 @@ export interface PipelineResult {
export function createRunner(options): Runner {
let runnerConstructor: (any) => Runner;
- if (options.runner == undefined || options.runner == "default") {
+ if (
+ options.runner === null ||
Review Comment:
No need to allow null.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17732: Clean up uses of == instead of === in ts sdk
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17732:
URL: https://github.com/apache/beam/pull/17732#issuecomment-1134755286
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] robertwb merged pull request #17732: Clean up uses of == instead of === in ts sdk
Posted by GitBox <gi...@apache.org>.
robertwb merged PR #17732:
URL: https://github.com/apache/beam/pull/17732
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17732: Clean up uses of == instead of === in ts sdk
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17732:
URL: https://github.com/apache/beam/pull/17732#issuecomment-1134755291
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17732: Clean up uses of == instead of === in ts sdk
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17732:
URL: https://github.com/apache/beam/pull/17732#issuecomment-1134755284
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on pull request #17732: Clean up uses of == instead of === in ts sdk
Posted by GitBox <gi...@apache.org>.
damccorm commented on PR #17732:
URL: https://github.com/apache/beam/pull/17732#issuecomment-1145216784
@robertwb would you mind merging?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on pull request #17732: Clean up uses of == instead of === in ts sdk
Posted by GitBox <gi...@apache.org>.
damccorm commented on PR #17732:
URL: https://github.com/apache/beam/pull/17732#issuecomment-1138585470
@robertwb would you mind taking a look?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org