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

fix nc branding #1

Merged
merged 5 commits into from
Aug 29, 2016
Merged

fix nc branding #1

merged 5 commits into from
Aug 29, 2016

Conversation

ChristophWurst
Copy link
Member

cc @nextcloud/mail

@@ -1,43 +1,42 @@
# Mail
Copy link
Member

Choose a reason for hiding this comment

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

Lets add "Nextcloud" here again 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

sound like Let's make Mail great again 😉

Copy link
Member

@MariusBluem MariusBluem Aug 29, 2016

Choose a reason for hiding this comment

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

FIRST THE CORE, THEN THE MAIL APP AND LAST BUT NOT LEAST AMERICA 😈😅😆
donald-trump-trademarked-a-ronald-reagan-slogan-and-would-like-to-stop-other-republicans-from-using-it
cc @ChristophWurst

@MariusBluem
Copy link
Member

Could we please put every screenshot into the screenshots repo 😁

@jancborchardt
Copy link
Member

Yup, Marius is right, the screenshots should not clutter up the app repos. :) I opened a pull request in the screenshots repo: nextcloud/screenshots#20

@ChristophWurst can you remove the screenshot from here and amend the commit?

@ChristophWurst
Copy link
Member Author

Yup, Marius is right, the screenshots should not clutter up the app repos. :) I opened a pull request in the screenshots repo: nextcloud/screenshots#20

@ChristophWurst can you remove the screenshot from here and amend the commit?

I don't think that would clutter the repo, it's just one file. Furthermore I find it quite handy to have it in the mail git repo as you can then go back and forth between releases and see the appropriate screenshot.

@irgendwie
Copy link
Member

irgendwie commented Aug 29, 2016

% grep -inrP '(?<!@)owncloud'

js/README.md:20:Generally, any code contributed to this repository should comply with the general ownCloud coding style guidelines.
[...]
lib/controller/aliasescontroller.php:5: * ownCloud - Mail
[...]

I think both lines should be adapted for Nextcloud! 👎

@jancborchardt
Copy link
Member

@irgendwie good find! :) Do you want to fix them and add a commit?

@MariusBluem
Copy link
Member

I don't think that would clutter the repo, it's just one file. Furthermore I find it quite handy to have it in the mail git repo as you can then go back and forth between releases and see the appropriate screenshot.

😕 ... I think it does not make sense to store a screenshot in the repo and a separate repo. We have also no screenshots in calendar/contacts or server repo. But thats only my opinion.
@ChristophWurst @jancborchardt

@@ -1,5 +1,5 @@
/**
* ownCloud - Mail
* Mail
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason of why we are not simply replacing that with Nextcloud - Mail ?! 😁 Does not matter anyway - but yeah 😅

@irgendwie
Copy link
Member

@jancborchardt Done - my grep filtering also shows some more occurrences of ownCloud, but I think we can ignore them for now.

@jancborchardt
Copy link
Member

I tend to agree with @Mar1u5 that we don’t really need to store the screenshots here as well. The screenshots repo has branches for releases as well and the look of the app doesn’t change that more often.

Anyway it’s your call @ChristophWurst. I adjusted the screenshot and compressed it.

@ChristophWurst
Copy link
Member Author

thanks for the fixes @jancborchardt @irgendwie. Does this PR qualify for a 👍 now? LGTM is waiting for an approval comment 😛

@irgendwie
Copy link
Member

Would say yes! LGTM 👍

@ChristophWurst ChristophWurst merged commit f56b8ea into master Aug 29, 2016
@ChristophWurst ChristophWurst deleted the fix-branding branch August 29, 2016 13:34
@jancborchardt
Copy link
Member

By the way, you are all invited to the Nextcloud conference, come register at https://conf.nextcloud.com :)
(If you didn't register yet or didn't submit a lightning talk yet. ;) cc @nextcloud/mail @Mar1u5 @zeugmatis

@zeugmatis
Copy link

Ah, I would totally go but I am in California! Just north of SF. Hope to try and sell NC here, but dropbox being free... is one tough competitor! Maybe same time next year. :-)

@jancborchardt
Copy link
Member

@zeugmatis ooh cool! Maybe start a Nextcloud meetup there then? ;)

@ghost ghost mentioned this pull request Oct 19, 2017
@lock
Copy link

lock bot commented Nov 21, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and questions.

@lock lock bot locked and limited conversation to collaborators Nov 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants