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

feature/sc-123396/implement-execute-command-with-eventdata #692

Conversation

hugomontero
Copy link
Contributor

@hugomontero hugomontero commented Nov 24, 2023

Description

This PR will add logic execute command in order to execute a logic function and get the logs and result

How to Test

  1. Pull down branch: git pull && git checkout feature/sc-123396/implement-execute-command-with-eventdata
  2. Install dependencies: npm i
  3. Run unit tests: npm run test:unit
  4. Attempt to execute a logic function previously created
  5. Attempt to execute a logic function that will fails (syntax error for instance)
  6. Attempt to execute a logic function in a dir with more that one .js files

Outcome

  • Tests should pass
  • On executing the logic function -> Execute will show logs output and status
  • On executing the logic function with syntax error -> Execute will show the error and status as execption
  • On executing more than one files inside -> Execute will prompt you which file you want to execute

Related Issues / Discussions

Story details: https://app.shortcut.com/particle/story/123396/implement-execute-command-with-eventdata

Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA
  • Problem and solution clearly stated
  • Tests have been provided
  • Docs have been updated
  • CI is passing

@hugomontero hugomontero force-pushed the feature/sc-123396/implement-execute-command-with-eventdata branch 4 times, most recently from de22a2e to 5c87b7b Compare December 4, 2023 15:26
@hugomontero hugomontero marked this pull request as ready for review December 4, 2023 16:35
@hugomontero hugomontero changed the title Feature/sc 123396/implement execute command with eventdata feature/sc-123396/implement-execute-command-with-eventdata Dec 4, 2023
@hugomontero hugomontero force-pushed the feature/sc-123396/implement-execute-command-with-eventdata branch from 5c87b7b to fa201f4 Compare December 4, 2023 20:34
Comment on lines 188 to 199
const logicDataBuffer = await fs.readFile(dataPath);
logicData = logicDataBuffer.toString();
Copy link
Member

Choose a reason for hiding this comment

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

You can convert to a string while reading the file

Suggested change
const logicDataBuffer = await fs.readFile(dataPath);
logicData = logicDataBuffer.toString();
logicData = await fs.readFile(dataPath, 'utf8');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done over here! 👉 15afd83

Comment on lines +201 to +216
event: {
event_data: logicData,
event_name: 'test_event',
device_id: '',
product_id: 0
},
Copy link
Member

Choose a reason for hiding this comment

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

This is too restrictive for CLI testing. We'll definitely want to allow passing device id, product id, event name and event data. We'll also want to be able to pass other types of triggers like scheduled and Ledger change. But for now that's ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. I wanted to put two params: --deviceId and --productId in order to make the user can test different devices/products according their needs. I didn't that now given that I wasn't sure the best ui interaction for asking that info.
Thinking about a prompt or just adding that into a option param

Base automatically changed from feature/sc-123395/implement-create-command to feature/logic-function December 5, 2023 16:12
@hugomontero hugomontero force-pushed the feature/sc-123396/implement-execute-command-with-eventdata branch from fa201f4 to 1312a53 Compare December 5, 2023 16:15
@hugomontero hugomontero merged commit 7930a5d into feature/logic-function Dec 5, 2023
6 checks passed
@hugomontero hugomontero deleted the feature/sc-123396/implement-execute-command-with-eventdata branch December 5, 2023 17:12
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.

None yet

2 participants