FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

chore: Remove the src directory from packages.json's files by sy-records · Pull Request #1344 · docsifyjs/docsify · GitHub

  1. docsifyjs
  2. docsify

Repository navigation

chore: Remove the src directory from packages.json's files - #1344

Merged
anikethsaha merged 2 commits into
docsifyjs:developfrom
sy-records:remove-src
Aug 20, 2020
ConversationCommits (2)ChecksFiles changed
Merged

chore: Remove the src directory from packages.json's files#1344
anikethsaha merged 2 commits into
docsifyjs:developfrom
sy-records:remove-src

Conversation

sy-records commented Aug 20, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

close #1343

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Repo settings
  • Other, please describe:

If changing the UI of default theme, please provide the before/after screenshot:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)

You have tested in the following browsers: (Providing a detailed version will be better.)

  • Chrome
  • Firefox
  • Safari
  • Edge
  • IE

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated
  • Related tests have been updated

To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.

vercel Bot commented Aug 20, 2020 •
edited
Loading

Copy link
Copy Markdown

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/docsify-core/docsify-preview/3g7nmuq1y
✅ Preview: https://docsify-preview-git-fork-sy-records-remove-src.docsify-core.vercel.app

vercel Bot temporarily deployed to Preview August 20, 2020 00:09 Inactive
sy-records requested review from a team and anikethsaha August 20, 2020 00:09

codesandbox-ci Bot commented Aug 20, 2020

Copy link
Copy Markdown

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit f40c1c4:

Sandbox Source
docsify-template Configuration

anikethsaha merged commit 5037f0b into docsifyjs:develop Aug 20, 2020
sy-records deleted the remove-src branch August 20, 2020 06:07
trusktr reviewed Aug 21, 2020
View reviewed changes

trusktr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is a breaking change. We should not remove src. Someone may depend on it. Why did you remove it?

trusktr commented Aug 21, 2020

Copy link
Copy Markdown
Member

@QingWei-Li Hello! We've been trying to save breaking changes for v5, but first we're stabilizing the code with tests (see Playwright PR, and some new files like docsify.test.js).

In my opinion, src files should always be published. This allows people to integrate source files into their own build systems which may be preferable over consuming built files (something I do quite often).

snyk-bot mentioned this pull request Aug 21, 2020
Closed

anikethsaha commented Aug 23, 2020

Copy link
Copy Markdown
Member

@trusktr IMO, I don't think we should publish that. is someone who wants to use the source file, they can very well pull it from the Github and also publishing the src folders will increase the package size unnecessary.

This is not a breaking change cause we never recommended nor stated anywhere in the docs to use these files and even if we did so, we should fix that.

snyk-bot mentioned this pull request Sep 12, 2020
Merged

This branch was successfully deployed

1 active deployment
Preview — f40c1c47 Deployed Aug 20, 2020 by vercel[bot]
View deployment
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

trusktr trusktr left review comments
QingWei-Li QingWei-Li approved these changes
anikethsaha anikethsaha approved these changes
jhildenbiddle Awaiting requested review from jhildenbiddle
Koooooo-7 Awaiting requested review from Koooooo-7

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove the src directory from packages.json's files

4 participants

Back | FazBrowse Home | New Git URL