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

[Vaadin Directory] Changed URI field star to stars #1597

Closed
wants to merge 14 commits into from
Closed

[Vaadin Directory] Changed URI field star to stars #1597

wants to merge 14 commits into from

Conversation

binbalenci
Copy link

No description provided.

@shields-ci
Copy link

Messages
📖

✨ Thanks for your contribution to Shields, @binhbbui411!

📖

Thanks for contributing to our documentation. We ❤️ our documentarians!

Generated by 🚫 dangerJS

Copy link
Member

@chris48s chris48s left a comment

Choose a reason for hiding this comment

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

Given this hasn't been deployed yet I think its reasonable to change the endpoint because every other service uses /stars as opposed to /star and there is no backwards-compatibility to break yet.

However, your pull request is difficult to review in its current form due to the unrelated commits and merge conflict. Could you fetch the latest upstream commits and rebase your changes onto master please?

@binbalenci
Copy link
Author

Thanks for the comment @chris48s. I have done all of the above steps. Please check the new pull request: #1603

Regards,
Binh

@paulmelnikow
Copy link
Member

Closing in favor of #1603.

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.

4 participants