Skip to content

Conversation

pniedzwiedzinski
Copy link
Contributor

Summary

This PR closes #1762

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

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • 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.

Other information:

@kefranabg
Copy link
Collaborator

Hi @pniedzwiedzinski,

Thanks for this PR. I’ll do a review ASAP.

@kefranabg
Copy link
Collaborator

@pniedzwiedzinski Could you revert changes related to the code formatting? It would remove unnecessary changes and would be easier to understand what is really related to the feature 😉

@flozero flozero added complexity: easy Easy complexity need feedback Awaiting author response status: community assigned Community assigned topic: config Relates to VuePress config version: 1.x Relates to version 1 of VuePress labels Sep 5, 2019
@pniedzwiedzinski pniedzwiedzinski changed the title [WIP] Fix #1762 Fix #1762 Sep 7, 2019
Copy link
Member

@ulivz ulivz left a comment

Choose a reason for hiding this comment

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

Since "lodash/isNil" is not a explicitly declared dependency in VuePress, you need to add it by yourself.

BTW, the title of a pull request should be given so we can know what problems it solves, Thanks.

@pniedzwiedzinski pniedzwiedzinski changed the title Fix #1762 Fix editLink on specific page Sep 9, 2019
Copy link
Collaborator

@kefranabg kefranabg left a comment

Choose a reason for hiding this comment

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

Thanks for your work! Could you add lodash in the theme-default package dependencies and solve the conflict?

@kefranabg
Copy link
Collaborator

@pniedzwiedzinski Is that ok for you to solve the conflict? If you're not comfortable with it I'll do it.

@pniedzwiedzinski
Copy link
Contributor Author

Better if you do that, thanks!

@kefranabg kefranabg changed the title Fix editLink on specific page fix($theme-default): Fix editLink on specific page Sep 12, 2019
@kefranabg kefranabg requested a review from flozero September 19, 2019 21:23
@kefranabg kefranabg merged commit 0e8a442 into vuejs:master Sep 20, 2019
@vue-bot
Copy link

vue-bot commented Sep 20, 2019

Hey @pniedzwiedzinski, thank you for your time and effort spent on this PR, contributions like yours help make Vue better for everyone. Cheers! 💚

@ulivz
Copy link
Member

ulivz commented Oct 11, 2019

When I'm reading the changelog, I found that this pull request doesn't match the commit message convention, and even took a misleading information.

Since it's actually a feature request instead of a bug fixes, we need to modify the prefix from fix: to feat: before squashing it. Also, (close: #1762) should be here which will help issue tracking. (// cc @f3ltron @kefranabg @newsbielt703 @bencodezen )

So the final commit message should be:

feat($theme-default): enable editLink on specific page via frontmatter (close: #1762) (#1825)

Comparing to the actual commit:

fix($theme-default): Fix editLink on specific page (#1825)

@pniedzwiedzinski pniedzwiedzinski changed the title fix($theme-default): Fix editLink on specific page feat($theme-default): enable editLink on specific page via frontmatter (close: #1762) (#1825) Oct 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: easy Easy complexity need feedback Awaiting author response status: community assigned Community assigned topic: config Relates to VuePress config version: 1.x Relates to version 1 of VuePress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable editLink on specific page
5 participants