Skip to content

/ write-javascript-actions

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

Jump to bottom

General feedback #4

Closed
hectorsector opened this issue Jan 9, 2020 · 4 comments
Closed

General feedback #4

hectorsector opened this issue Jan 9, 2020 · 4 comments
Labels
enhancement
Projects
Learning Lab Course maintenance

Comments

@hectorsector
Copy link
Member

@hectorsector hectorsector commented Jan 9, 2020

Overall

  • Loved this course! I learned so much and I feel like it can be leveraged internally as well as publicly.
  • The jokes were a fantastic way to end the course and really liven it up
  • Let's think about ways to break up large blocks of content, and be deliberate about the portions that are course outcomes (like defining actions metadata) versus those that are just prereqs (like understanding how to use a try/catch block)
  • I'll help with error handling ✋
  • I pushed up some direct changes over in #3, if you're ok with those @mattdavis0351 let's merge them in.

In 05_explain-metadata.md:

  • I'm not sure I understand why it's mentioning this at all:

    It's not looking for a JavaScript file or a config.yml file.

  • it's missing an explanation of why the metadata is necessary at all, something like the docs might help here:

    The data in the metadata file defines the inputs, outputs and main entrypoint for your action.

In 05_create-metadata.md:

  • We may want to rely on formal YAML definitions to minimize confusion and allow people to Google the terminology if they don't understand it. For example, replace "Lastly, define the run parameter to use "node12" to execute the "main.js"" with:

    Create a runs dictionary with the following key-value pairs: using: "node12" and main: "main.js"

  • Alternatively, we can use the descriptions as they currently exist, and just refer folks back to the syntax of the actions.yml file in the docs for reference -- I realize we're doing this already, but we should do it again right here where it's needed.

In 07_add-input-params.md:

  • we may want to link directly to the getInput() part of the doc

In 10_action-two.md:

  • is adding the modules folder necessary? It's a pretty bulky add and push
  • I suggest referencing files by their full path, as I didn't heed the warning to do it in the proper path the first time I read it:
    1. Create a new folder for our actions files:
      mkdir joke-action
      The full path should be .github/actions/joke-action

In 14_create-js-files.md

  • it's a lot of content at once, let's think about how to break this up
  • the screenshot in place of a fenced code block is tough to work with
@mattdavis0351
Copy link
Contributor

@mattdavis0351 mattdavis0351 commented Jan 9, 2020

@hectorsector Thanks a ton, I'm glad you enjoyed it. I'll look through this a bit more thoroughly but there are two things that stand out right away to me that I can address.

is adding the modules folder necessary? It's a pretty bulky add and push

Unfortunately, yes it has to be pushed. I show at the very end how to use ncc to avoid doing that in the future, but took the stance that compiling JavaScript was outside of the scope of how to write an action.

There are dependencies that the action relies on that have to be uploaded for it to work.

I suggest referencing files by their full path, as I didn't heed the warning to do it in the proper path the first time I read it:

I'm struggling with figuring out the "right" way to do this. At first I created a step that was
mkdir .github/actions/joke-action and what ended up happening is that my alpha-tester made ended up with a directory like this .github.com/action/first-action/.github/actions/joke-action because they just copied and pasted and their current directory was from the previous steps.

I'm not sure the best way to ensure our learners are in the proper path without things getting confusing in the instructions.
@hectorsector
Copy link
Member Author

@hectorsector hectorsector commented Jan 10, 2020

is adding the modules folder necessary? It's a pretty bulky add and push

Unfortunately, yes it has to be pushed

👍 thanks for the confirmation, @mattdavis0351

I suggest referencing files by their full path, as I didn't heed the warning to do it in the proper path the first time I read it:

I'm struggling with figuring out the "right" way to do this. At first I created a step that was
mkdir .github/actions/joke-action and what ended up happening is that my alpha-tester made ended up with a directory like this .github.com/action/first-action/.github/actions/joke-action because they just copied and pasted and their current directory was from the previous steps.

I think removing the ambiguity would help the most learners. We can also make sure the files are in the right paths with gates, I'll help with that.
@hectorsector hectorsector added this to Triage in Learning Lab Course maintenance via automation Jul 14, 2020
@davenowell davenowell moved this from Triage to To do: suggestion or enhancement in Learning Lab Course maintenance Jul 28, 2020
@brianamarie
Copy link
Member

@brianamarie brianamarie commented Jul 29, 2020

Hello @hectorsector @mattdavis0351 👋 Are there still aspects of this feedback that should be included?
@hectorsector
Copy link
Member Author

@hectorsector hectorsector commented Jul 29, 2020

I think most of it is addressed. We can open individual issues for anything new that might arise.
Learning Lab Course maintenance automation moved this from To do: suggestion or enhancement to Done Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
Learning Lab Course maintenance
  
Done
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
@brianamarie @davenowell @hectorsector @mattdavis0351
You can’t perform that action at this time.