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

feat: Various improvements for electron/template #950

Merged
merged 6 commits into from
Jun 20, 2019

Conversation

felixrieseberg
Copy link
Member

  • I have read the contribution documentation for this project.
  • I agree to follow the code of conduct that this project follows, as appropriate.
  • The changes are appropriately documented (if applicable).
  • The changes have sufficient test coverage (if applicable).
  • The testsuite passes successfully on my local machine (if applicable).

Summarize your changes:

  1. Updated the core .gitignore file to cover most common Node.js tools, ensuring that nobody gets caught by surprise check-in files
  2. Added a README for the webpack template. It's currently the same as for electron/template, but maybe that's okay?
  3. Documented renderer.js a little bit, explaining what's what
  4. Made the index.html a bit prettier 😉

Screen Shot 2019-06-18 at 10 19 13 AM

.webpack/

# Electron-Forge
out/
Copy link
Member

Choose a reason for hiding this comment

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

Ah, the Node.gitignore...

🤔 it'd be interesting if this were automatically updated with a GitHub Action...

* [Webpack Documentation](https://webpack.js.org/)
* [Node.js Documentation](https://nodejs.org/en/docs/)

[generate]: https://github.com/electron/template/generate
Copy link
Member

Choose a reason for hiding this comment

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

I don't know whether this fits in the Webpack template.

Copy link
Member

Choose a reason for hiding this comment

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

Let's just keep this in electron/template only for now.

padding: 2rem;
margin: auto;
}
</style>
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this should go in a separate CSS file, mostly to show that non-transpiled files work fine with Forge+Webpack. There was a question about it in the issue tracker the other day.

Copy link
Member Author

@felixrieseberg felixrieseberg Jun 19, 2019

Choose a reason for hiding this comment

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

Sure, sure - I'd say let's incrementally improve this as much as there's time and appetite! If you think this is better than what's there right now, let's go with this first - I'm just trying to get electron/template 🚢

packages/api/core/tmpl/index.html Outdated Show resolved Hide resolved
felixrieseberg and others added 2 commits June 19, 2019 12:35
Co-Authored-By: Mark Lee <malept@users.noreply.github.com>
@malept malept merged commit 641f521 into master Jun 20, 2019
@malept malept deleted the template-improvements branch June 20, 2019 15:10
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.

2 participants