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

JST-652: Implement coding conventions #723

Merged
merged 5 commits into from
Dec 15, 2023

Conversation

grisha87
Copy link
Contributor

This PR implements the agreed coding conventions:

  • private members cannot be prefixed with _
  • we don't use poperty getters/setters - switched to setX, getX accessors
  • the primary naming style is camelCase with UPPER_CASE and PascalCase in other cases

@grisha87 grisha87 changed the base branch from master to beta December 14, 2023 13:01
Copy link
Contributor

@mgordel mgordel left a comment

Choose a reason for hiding this comment

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

I didn't notice anything incorrect

await this.yagna.connect();
this.api = this.yagna.getApi();

this.initializedState = true;
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 we need all three api, yagna and initializedState, can't we achieve the same business logic with just api?

public async init() {
    if (this.isInitialized()) {
      return;
    }
    const yagna = new Yagna(this.config.yagna);
    // this will throw an error if yagna is not running
    await yagna.connect();
    this.api = this.yagna.getApi();
}

public isInitialized() {
  return this.api !== null
}

import { RunJobOptions } from "../job/job";
import { GolemError } from "../error/golem-error";

type GolemNetworkConfig = Partial<RunJobOptions> & { yagna?: YagnaOptions };
Copy link
Contributor

Choose a reason for hiding this comment

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

good idea with the type, can we export it in the root index.ts?

@grisha87 grisha87 requested a review from SewerynKras December 15, 2023 11:49
@grisha87 grisha87 force-pushed the task/JST-652/implement-coding-conventions branch from 562ab6a to 7b47d58 Compare December 15, 2023 12:20
@grisha87 grisha87 force-pushed the task/JST-652/implement-coding-conventions branch from 7b47d58 to 50cb9d8 Compare December 15, 2023 13:10
@grisha87 grisha87 merged commit 6731207 into beta Dec 15, 2023
8 checks passed
@grisha87 grisha87 deleted the task/JST-652/implement-coding-conventions branch December 15, 2023 13:26
Copy link

🎉 This PR is included in version 2.0.0-beta.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Copy link

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants