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

Guides Cleanup #1337

Merged
merged 2 commits into from
Jun 29, 2017
Merged

Guides Cleanup #1337

merged 2 commits into from
Jun 29, 2017

Conversation

skipjack
Copy link
Collaborator

@skipjack skipjack commented Jun 24, 2017

This pull request...

  • Removes two guides which were incomplete and only had TODOs
  • Moves the comparison.md page as it doesn't really fall under guides

See the commit messages for more details on the "why" behind these changes.

cc @TheDutchCoder @jakearchibald

We can choose to revive these later if they are requested by a significant
amount of people. Also, especially the content in compatibility.md, is covered
in other places (specifically in the `module-***.md` pages in `/api`).
This isn't really as much of an instructional as it is just information about
different bundling and build systems.
@skipjack
Copy link
Collaborator Author

@bebraw the build errors can be ignored as the unpkg 500 should now be fixed and the other one is just a missing edit link for the moved comparison guide.

@@ -109,7 +109,7 @@ __src/index.js__
+
function component() {
var element = document.createElement('div');

- // Lodash, currently included via a script, is required for this line to work
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment was on purpose, as it shows the user that in this case lodash is imported through a <script> tag, whereas later on it's imported by webpack.

Would you mind leaving it in (or maybe clarify it)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah yeah sorry about that. I think I either misunderstood or took this out accidentally. Will remove this change tonight.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@TheDutchCoder just double-checked this. It's a bit confusing because your viewing a code block of the syntax highlighting language diff within a GitHub change diff. It tripped me up too when I first saw your comment. The diff from source tree is easier to understand:

image

It's only the blank line above that comment that changed. Just a minor editorconfig autofix 👍 . Your comment should remain untouched.

@TheDutchCoder
Copy link
Collaborator

TheDutchCoder commented Jun 26, 2017 via email

@skipjack skipjack merged commit 690eb55 into master Jun 29, 2017
@skipjack skipjack deleted the guides-cleanup branch June 29, 2017 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants