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

Prepare for release #99

Merged
merged 35 commits into from
Jun 18, 2021
Merged

Prepare for release #99

merged 35 commits into from
Jun 18, 2021

Conversation

lilsweetcaligula
Copy link
Collaborator

@lilsweetcaligula lilsweetcaligula commented Jun 9, 2021

@lilsweetcaligula lilsweetcaligula self-assigned this Jun 9, 2021
@lilsweetcaligula lilsweetcaligula marked this pull request as draft June 9, 2021 07:29
@lilsweetcaligula lilsweetcaligula marked this pull request as ready for review June 9, 2021 17:34
@lilsweetcaligula lilsweetcaligula marked this pull request as draft June 9, 2021 17:34
@lilsweetcaligula lilsweetcaligula marked this pull request as ready for review June 9, 2021 18:58

jobs:
build:
timeout-minutes: 4
Copy link
Collaborator Author

@lilsweetcaligula lilsweetcaligula Jun 10, 2021

Choose a reason for hiding this comment

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

Having a timeout is very important. For example, on my personal account yesterday, while I was testing a GitHub actions setup, I had a test suite hang. That used up all of free tier minutes for my account which caught me completely unawares - the default timeout is 6 hours.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yikes - good to know

@lilsweetcaligula lilsweetcaligula marked this pull request as ready for review June 10, 2021 05:31

jobs:
build:
timeout-minutes: 4
Copy link
Collaborator

Choose a reason for hiding this comment

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

yikes - good to know

.github/workflows/build.yml Show resolved Hide resolved
.github/workflows/build.yml Show resolved Hide resolved
lib/intern.js Outdated
const qq = {}

for (const qp in q) {
if (is_seneca_qualifier(qp)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

use "directive", not "qualifier"
"directive" is the terminology established in the Seneca docs for $ props that do special things

lib/intern.js Outdated
}


if (is_mongo_qualifier(qp)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

and mongo must have their own term for "qualifier" too - see if you can find a reference in the mongo docs

mongo-store.js Outdated

/*
native$ = object => use object as query, no meta settings
native$ = array => use first elem as query, second elem as meta settings
*/

module.exports = function (opts) {
const mongoStore = function (opts) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

use:
const mongo_store = function mongo_store(options) {

this updates the names to be consistent with standard usage:

  • the name of the plugin function is significant - Seneca uses if for the plugin name in logs (until overridden by exports)
  • plugin options should always be called options for consistency
  • the const variable should have the same name as the function

mongo-store.js Show resolved Hide resolved
mongo-store.js Show resolved Hide resolved
mongo-store.js Show resolved Hide resolved
@rjrodger rjrodger merged commit 298f8ba into master Jun 18, 2021
@lilsweetcaligula lilsweetcaligula deleted the prepare-for-release branch June 18, 2021 09:51
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