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

Expand "Local development" #363

Closed
wants to merge 9 commits into from
Closed

Expand "Local development" #363

wants to merge 9 commits into from

Conversation

tbeseda
Copy link
Member

@tbeseda tbeseda commented Aug 2, 2021

🚧 WIP: creating draft PR to coordinate with Ryan, but all feedback welcome :)

Changes

Expand "Local development"

I added an example for "Creating a new resource". I used a hypothetical "hit counter" example. arc init is used for scaffolding (mentioned in this issue)
This type of workflow was a point of interest as I was building my first app.

Stub out "Debugging" section that may or may not be necessary. Leads into testing.

Move "Testing" to its own document

Testing is definitely a part of "local dev" but probably warrants its own doc.

More ideas for "Local development"

  • provide a short implementation of the "hit counter" event feature, just to tie it all together
  • use inline tabs to demonstrate Ruby and Python alongside the Node example
  • elaborate on what Architect Sandbox emulates locally (and how?)
    • ie. queues, events, dynalite
    • this is covered in reference/cli/sandbox, but there might be an opportunity to tout some of the capabilities without requiring the reader to dig too far.

I avoided second person, but after research on style guides by large orgs, I'll change it.


  • Forked the repo and created your branch from master
  • Made sure tests pass (run npm it from the repo root)
  • Updated relevant documentation internal to this repo (e.g. readme.md, help docs, inline docs & comments, etc.)
  • Linked to any related issues, PRs, etc. below that may relate to, consume, or necessitate these changes

tbeseda added 5 commits July 30, 2021 18:28
commit 84f64f5
Author: Taylor Beseda <[email protected]>
Date:   Sat Jul 31 14:37:33 2021 -0600

    add http smoke test

commit 3c896bd
Author: Taylor Beseda <[email protected]>
Date:   Sat Jul 31 13:45:56 2021 -0600

    update call to highlighter.js#highlight

    avoid deprecation notices per highlightjs/highlight.js#2277

commit 1c8f86b
Author: Taylor Beseda <[email protected]>
Date:   Sat Jul 31 13:24:23 2021 -0600

    use @architect/asap to proxy static playground

    in lieu of @architect/functions http.proxy

    this became an issue when dependabot updated functions to 4.0.0
│ ├── index.js
│ └── config.arc
└── app.arc
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, some funky unicode artifacts here?
image

Copy link
Member Author

Choose a reason for hiding this comment

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

Super weird. that line is the same as the other └── app.arc instances in project-layout.md and queues.md.
and the character that shows up garbled in your screenshot is used in each of the lines above the app.arc line in your screenshot.

The issue must be between the code and the HTML output, maybe the Markdown compiler. but I'm having a hard time reproducing that garbled text.
I'm already taking a look there since there seems to be an issue with my highlighter.js change, so I can take some time to look more at this.

Copy link
Member

@filmaj filmaj left a comment

Choose a reason for hiding this comment

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

Wow, this is so awesome! Thanks for taking this on! I think this is a massive improvement!

I left a few comments, mostly around wording and dropped a few suggestions around linking to other parts of the docs from relevant areas.

Thank you so much for sending this PR!

@tbeseda
Copy link
Member Author

tbeseda commented Aug 4, 2021

Awesome feedback @ryanblock and @filmaj
I'm going to synthesize this today and update here this afternoon.
Thanks for taking a look.

tbeseda and others added 4 commits August 4, 2021 12:59
Thanks for the edits. going to normalize all linking in this doc next

Co-authored-by: Fil Maj <[email protected]>
also add "external" link icon with added markdown-it-attrs plugin
@tbeseda
Copy link
Member Author

tbeseda commented Aug 8, 2021

I included suggested edits and decided to use the project's main package.json when demonstrating dependency management.

I also added markdown-it-attrs in order to add target="_blank" to links that leave arc.codes. This will also be useful for adding other attributes while continuing to stick with Markdown instead of too much HTML.
also added a bit of CSS to show an icon next to external links. I can update all external links in a future PR if maintainers like this idea.

external link

Last, Ryan mentioned an issue with the encoding on characters used in the folder structure diagram (see his screenshot above). I wasn't able to reproduce this. I tried with Node 12, 14, and 16. Also fresh installs of the repo after clearing my npm cache.
It may relate to the occasional refresh issue I've seen when the sandbox restarts and the browser uses locally cached assets. Maybe another contributor can pull and boot up the app to check.

@tbeseda
Copy link
Member Author

tbeseda commented Sep 13, 2021

I'm closing this as we're making more targeted passes on docs. I will maintain the branch as a source for improvements to the text of the doc. Some changes here add dependencies and might be out of scope in the near future.

@tbeseda tbeseda closed this Sep 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants