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

feat(datastore-lock)!: factoring out datastore-lock #1764

Merged
merged 1 commit into from
May 13, 2021

Conversation

tmatsuo
Copy link
Contributor

@tmatsuo tmatsuo commented May 13, 2021

This small library is initially implemented in blunderbuss, but it should be useful for some other bots, so I'm trying to make it a npm package.

@tmatsuo tmatsuo requested review from bcoe and a team May 13, 2021 09:47
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 13, 2021
Copy link
Contributor

@chingor13 chingor13 left a comment

Choose a reason for hiding this comment

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

Will want @bcoe's feedback on naming. We can set up the release config in a followup PR.

return false;
}

private async _acquire(): Promise<AcquireResult> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Fun fact, I learned recently that private fields aren't actually private outside of the context of TypeScript, so when I actually wanted private methods, I ended up using symbols as method names:

https://github.com/yargs/yargs/blob/master/lib/yargs-factory.ts#L88

I'm not suggesting we change anything, just wanted to share this surprising TypeScript behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah it makes sense.

Copy link
Contributor Author

@tmatsuo tmatsuo left a comment

Choose a reason for hiding this comment

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

Changed the package name to @github-automations/datastore-lock.
@bcoe PTAL

@tmatsuo tmatsuo added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 13, 2021
@tmatsuo
Copy link
Contributor Author

tmatsuo commented May 13, 2021

I'll add release config first :)

@tmatsuo tmatsuo changed the title feat(datastore-lock): factoring out datastore-lock feat(datastore-lock)!: factoring out datastore-lock May 13, 2021
@tmatsuo tmatsuo removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 13, 2021
@tmatsuo
Copy link
Contributor Author

tmatsuo commented May 13, 2021

I'm trying to rewrite the first commit message to be a breaking change, but my local git client drops the exclamation mark.

I'm going to squash locally and re-push the change with a new commit message.

* changed the namespace to github-automations
* changed the version to 0.1.0
Copy link
Contributor Author

@tmatsuo tmatsuo left a comment

Choose a reason for hiding this comment

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

@bcoe I guess I'm ready for the release, PTAL

@bcoe bcoe merged commit c7072d0 into googleapis:master May 13, 2021
@release-please release-please bot mentioned this pull request May 13, 2021
@release-please release-please bot mentioned this pull request Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants