kredits/contracts
9
1
Fork 0
Code Issues 21 Pull Requests Actions Projects 1 Releases 8 Activity

Improve helper scripts #41

Merged
bumi merged 7 commits from feature/improve-scripts into master 2018-04-29 07:43:23 +00:00
Conversation 13 Commits 7 Files Changed 7
+111 -51
bumi commented 2018-04-23 10:12:34 +00:00 (Migrated from github.com)
Copy Link
Copy Source

now using the library in add-proposal.js, too. And the CLI is smarter and allows calling functions on the contract and on the wrapper.

now using the library in add-proposal.js, too. And the CLI is smarter and allows calling functions on the contract and on the wrapper.
fsmanuel (Migrated from github.com) reviewed 2018-04-25 21:42:12 +00:00
fsmanuel (Migrated from github.com) left a comment
Copy Link
Copy Source

Some comments

Some comments
scripts/add-contributor.js
fsmanuel (Migrated from github.com) commented 2018-04-25 21:14:44 +00:00
Copy Link
Copy Source

Do you have a linter now?

Do you have a linter now?
scripts/add-proposal.js
fsmanuel (Migrated from github.com) commented 2018-04-25 21:20:35 +00:00
Copy Link
Copy Source

Where does contributors come from?

Where does `contributors` come from?
scripts/cli.js
fsmanuel (Migrated from github.com) commented 2018-04-25 21:31:18 +00:00
Copy Link
Copy Source

So good that we went with upper case contract names and the properties instead of the functions!
But it will breaks on Contributor, wouldn’t it? I think we should rename that contract next and align the module code to it. Will make the future much brighter!

So good that we went with upper case contract names and the properties instead of the functions! But it will breaks on `Contributor`, wouldn’t it? I think we should rename that contract next and align the module code to it. Will make the future much brighter!
scripts/cli.js
@@ -30,2 +30,4 @@
return;
}
let argumentInput = await promptly.prompt('Arguments (comma separated): ', { default: '' });
let args = [];
fsmanuel (Migrated from github.com) commented 2018-04-25 21:32:13 +00:00
Copy Link
Copy Source

👀 nice!

👀 nice!
scripts/cli.js
@@ -34,1 +44,4 @@
func = contractWrapper.functions[method];
}
func.apply(contractWrapper, args).then((result) => {
console.log("\nResult:");
fsmanuel (Migrated from github.com) commented 2018-04-25 21:38:20 +00:00
Copy Link
Copy Source

Haha master of es6. I think you don’t need the if and the initial args
let args = argumentInput.split(',') should return an empty array.

Haha master of es6. I think you don’t need the if and the initial `args` `let args = argumentInput.split(',')` should return an empty array.
bumi (Migrated from github.com) reviewed 2018-04-26 00:27:12 +00:00
scripts/add-contributor.js
bumi (Migrated from github.com) commented 2018-04-26 00:27:12 +00:00
Copy Link
Copy Source

nah, not yet. :(

nah, not yet. :(
bumi (Migrated from github.com) reviewed 2018-04-26 00:29:37 +00:00
scripts/cli.js
@@ -34,1 +44,4 @@
func = contractWrapper.functions[method];
}
func.apply(contractWrapper, args).then((result) => {
console.log("\nResult:");
bumi (Migrated from github.com) commented 2018-04-26 00:29:37 +00:00
Copy Link
Copy Source

oh yeah, true.

oh yeah, true.
bumi (Migrated from github.com) reviewed 2018-04-26 00:33:31 +00:00
scripts/cli.js
@@ -34,1 +44,4 @@
func = contractWrapper.functions[method];
}
func.apply(contractWrapper, args).then((result) => {
console.log("\nResult:");
bumi (Migrated from github.com) commented 2018-04-26 00:33:31 +00:00
Copy Link
Copy Source

ah noo. it returns an array with an empty string, that was the reason for the if. or what am I missing?

ah noo. it returns an array with an empty string, that was the reason for the if. or what am I missing?
bumi (Migrated from github.com) reviewed 2018-04-26 00:34:37 +00:00
scripts/cli.js
@@ -29,0 +25,4 @@
method = await promptly.prompt('Function: ');
}
if (!contractWrapper[method] && !contractWrapper.functions[method]) {
bumi (Migrated from github.com) commented 2018-04-26 00:34:37 +00:00
Copy Link
Copy Source

yes, agree on renaming it! and maybe adding an alias for Contributor <=> Contributors

yes, agree on renaming it! and maybe adding an alias for Contributor <=> Contributors
fsmanuel (Migrated from github.com) reviewed 2018-04-26 10:35:51 +00:00
scripts/cli.js
@@ -34,1 +44,4 @@
func = contractWrapper.functions[method];
}
func.apply(contractWrapper, args).then((result) => {
console.log("\nResult:");
fsmanuel (Migrated from github.com) commented 2018-04-26 10:35:51 +00:00
Copy Link
Copy Source

Ah ok.

Ah ok.
raucao (Migrated from github.com) reviewed 2018-04-26 10:40:37 +00:00
scripts/cli.js
@@ -34,1 +44,4 @@
func = contractWrapper.functions[method];
}
func.apply(contractWrapper, args).then((result) => {
console.log("\nResult:");
raucao (Migrated from github.com) commented 2018-04-26 10:40:37 +00:00
Copy Link
Copy Source

But if input is already not an empty string, wouldn't it return an array with one argument string?

But if input is already not an empty string, wouldn't it return an array with one argument string?
fsmanuel (Migrated from github.com) reviewed 2018-04-26 10:43:41 +00:00
scripts/cli.js
@@ -29,0 +25,4 @@
method = await promptly.prompt('Function: ');
}
if (!contractWrapper[method] && !contractWrapper.functions[method]) {
fsmanuel (Migrated from github.com) commented 2018-04-26 10:43:41 +00:00
Copy Link
Copy Source

I opened #44 for the alias

I opened #44 for the alias
bumi (Migrated from github.com) reviewed 2018-04-26 11:22:18 +00:00
scripts/cli.js
@@ -34,1 +44,4 @@
func = contractWrapper.functions[method];
}
func.apply(contractWrapper, args).then((result) => {
console.log("\nResult:");
bumi (Migrated from github.com) commented 2018-04-26 11:22:18 +00:00
Copy Link
Copy Source

https://jsbin.com/qarikabane/edit?js,console

it would return [""] but if no arguments are given I need [].

https://jsbin.com/qarikabane/edit?js,console it would return `[""]` but if no arguments are given I need `[]`.
raucao (Migrated from github.com) reviewed 2018-04-26 11:36:42 +00:00
scripts/cli.js
@@ -34,1 +44,4 @@
func = contractWrapper.functions[method];
}
func.apply(contractWrapper, args).then((result) => {
console.log("\nResult:");
raucao (Migrated from github.com) commented 2018-04-26 11:36:42 +00:00
Copy Link
Copy Source

I was actually asking about the trim inside, but no idea why. It's for removing whitespace, so that's still needed.

I was actually asking about the `trim` inside, but no idea why. It's for removing whitespace, so that's still needed.
bumi (Migrated from github.com) reviewed 2018-04-26 21:19:06 +00:00
scripts/cli.js
@@ -34,1 +44,4 @@
func = contractWrapper.functions[method];
}
func.apply(contractWrapper, args).then((result) => {
console.log("\nResult:");
bumi (Migrated from github.com) commented 2018-04-26 21:19:06 +00:00
Copy Link
Copy Source

yep, trim() for potential whitespace as for example in "first, second"

yep, trim() for potential whitespace as for example in `"first, second"`
bumi commented 2018-04-26 21:19:50 +00:00 (Migrated from github.com)
Copy Link
Copy Source

mergeable? or is anything missing?

mergeable? or is anything missing?
fsmanuel (Migrated from github.com) approved these changes 2018-04-29 07:43:13 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
raucao
bumi
fsmanuel
Labels
Clear labels
good first issue
Good for newcomers
 ipfs
rsk
scaling
bug
Something is not working
 dev environment
Config, builds, CI, deployment, etc.
 docs
Documentation
 duplicate
This issue or pull request already exists
 enhancement
Improving existing functionality
 feature
New functionality
 idea
Something to consider
 invalid
Not a bug
 kredits-1
Small contribution
 kredits-2
Medium contribution
 kredits-3
Large contribution
 question
Looking for an answer
release
major
release
minor
release
patch
security
All your base are belong to us
 ui/ux
User interface, process design, etc.
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
bkero (bkero) bumi (bumi) fsmanuel (fsmanuel) galfert (galfert) greg (Greg) haythem (haythem) hueso (hueso) raucao (Râu Cao) slvrbckt (Nick Jennings)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kredits/contracts#41