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

Add Shaark #1864

Merged
merged 3 commits into from
Nov 16, 2019
Merged

Add Shaark #1864

merged 3 commits into from
Nov 16, 2019

Conversation

MarceauKa
Copy link
Contributor

No description provided.

README.md Outdated Show resolved Hide resolved
@n8225 n8225 added the addition label Oct 28, 2019
Copy link
Member

@nodiscc nodiscc left a comment

Choose a reason for hiding this comment

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

@MarceauKa is there a reason why you deleted the pull request template text? It is meant to be completed/boxes checked - the syntax looks good anyway.

Also, isn't the choice of name unfortunate? I think it creates confusion with the original Shaarli, maybe find a name of your own? :) (as I work on Shaarli I will not be approving/refusing this patch as there is a conflict of interest. your app looks good though)

@MarceauKa
Copy link
Contributor Author

Hi @nodiscc! Sorry I didn't see that the template text is meant to stay 😅
I use Shaarli since its beginning (the sebsauvage version) and just for the challenge I built it with Laravel but it have lot more features (archiving, stories, passwords, 2fa, ...). But yeah, it can be confusing, personnaly I don't it's a problem.
I'll adjust the PR accordingly to your feedback.

@MarceauKa MarceauKa changed the title Add Laravel Shaarli Add Shaark Nov 14, 2019
@MarceauKa MarceauKa requested a review from n8225 November 15, 2019 07:49
@nodiscc nodiscc merged commit f64960a into awesome-selfhosted:master Nov 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants