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.

Sign up for GitHub

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

ATTN: Breaking changes proposed for v0.4.x #119

Closed
jimafisk opened this issue Jan 15, 2021 · 4 comments
Closed

ATTN: Breaking changes proposed for v0.4.x #119

jimafisk opened this issue Jan 15, 2021 · 4 comments
Labels
breaking Change that's not backwards compatible with previous versions

Comments

@jimafisk
Copy link
Member

I'm fairly convinced these would be the right calls:

  • The layout folder to become layouts
    • This is consistent with Hugo's directory structure
    • Plural naming convention is used with Plenti's other folders: assets, themes
  • The allComponents magic prop to become allLayouts
    • Better describes the items it holds
  • The types key in plenti.json for route overrides to become routes
    • Better describes the action that is being taken
  • Removing nodejs build process completely
    • No more ejectable build.js file
    • No more --nodejs=true flag for build/serve commands
    • It's becoming too difficult to maintain two separate build process
  • The --dir flag to become --output (Better flag name for output directory than --dir #101)
    • Better explains what the outcome is

I also thought about the following, but ultimately might just keep things the same:

  • Component signatures (e.g. layout_global_html_svelte) could be shortened to remove the layout_ prefix and _svelte suffix since that exists on every signature. I'm on the fence about this, it would be quicker to write, but maybe not as clear exactly what it is? If it stays the way it is now, the prefix will be changed to layouts_ per the updated folder structure above.
  • We use :field(title) in route overrides in plenti.json but content.fields.title in our svelte templates (coming from public/spa/ejected/content.js). We could make both plural or singular for consistency. I could see a case for either, there are multiple fields in the object that you can access, but you are currently only accessing one field in each case.
    • Hugo's .Params is a similar concept and is plural
@jimafisk
Copy link
Member Author

jimafisk commented Jan 19, 2021 •
edited
Loading

I'm also thinking about removing all shorthand for flags (e.g. -b for --benchmark, -B for --build, -p for --port, etc). I know that's pretty conventional for CLI tools to do this, but when people use this in docs or reference projects I think it obfuscates what is actually happening and puts up a barrier for someone new to the project.

Edit: Some good best practices: https://softwareengineering.stackexchange.com/questions/307467/what-are-good-habits-for-designing-command-line-arguments. Leads me to think it might be worth keeping single letter shorthand standard flags (e.g. -h for --help and -v for --version).

@jimafisk jimafisk mentioned this issue Jan 21, 2021
Merged
@jimafisk jimafisk added the breaking Change that's not backwards compatible with previous versions label Jan 24, 2021
@jimafisk jimafisk pinned this issue Feb 15, 2021
@jimafisk
Copy link
Member Author

In the router we create a prop called route that we pass to the top level <Html /> component: https://github.com/plentico/plenti/blob/13e13af5049c622e097a244b06972671ee4df9ee/ejected/router.svelte. We should rename this layout since it's really a class constructor for a template file, and that would fit better with the content => allContent and soon to be layout => allLayouts naming convention.

jimafisk added a commit that referenced this issue Mar 22, 2021
@jimafisk
Remove NodeJS builds (#119).
01891d1
@jimafisk jimafisk mentioned this issue Mar 22, 2021
Merged
jimafisk added a commit that referenced this issue Mar 22, 2021
@jimafisk
Rename allComponents to allLayouts (#119).
a801735
jimafisk added a commit that referenced this issue Mar 22, 2021
@jimafisk
Rename Types to Routes for path overrides (#119).
a2b37d8
jimafisk added a commit that referenced this issue Mar 22, 2021
@jimafisk
Rename --dir flag to --output (#119).
3ada512
jimafisk added a commit that referenced this issue Mar 22, 2021
@jimafisk
Rename layout/ folder to layouts/ (#119).
24b7377
jimafisk added a commit that referenced this issue Mar 22, 2021
@jimafisk
Rename prop: route => layout (#119).
2c619b1
jimafisk added a commit that referenced this issue Mar 22, 2021
@jimafisk
Rename :field() route replacement pattern to :fields() (#119).
883c9a1
@jimafisk
Copy link
Member Author

jimafisk commented Mar 22, 2021 •
edited
Loading

I made all the updates above, except I left full layout signatures for now (e.g. layouts_content_pages_svelte). Mainly this is just easier to grep search the codebase for layouts_ or _svelte, which isn't the best reason ever but we can always revisit.

I'll update the documentation as well:

layout => layouts:

types => routes:

:field() => :fields():

route => layout:

allComponents => allLayouts:

--dir => --output:

Remove --nodejs=true build flag: https://plenti.co/docs/github-actions

jimafisk added a commit that referenced this issue Mar 22, 2021
@jimafisk
Merge pull request #147 from plentico/issue-119
e1a6189 
Prep v0.4 release (#119).
@jimafisk jimafisk mentioned this issue Mar 22, 2021
Closed
@jimafisk jimafisk mentioned this issue Mar 30, 2021
Merged
@jimafisk
Copy link
Member Author

jimafisk commented Apr 2, 2021

Docs have been updated: https://plenti.co/docs

Pages for the old api, like https://plenti.co/docs/allcomponents, should now have a deprecation notice.

Screenshot of example deprecation notice

deprecation_notice

@jimafisk jimafisk unpinned this issue May 6, 2021
@jimafisk jimafisk mentioned this issue Jan 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
breaking Change that's not backwards compatible with previous versions
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@jimafisk