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.

Sign up for GitHub

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

Jump to bottom

use folders in CMake #3286

Merged
merged 1 commit into from
May 18, 2021
Merged

use folders in CMake #3286

merged 1 commit into from
May 18, 2021

Conversation

Dimon4eg
Copy link
Contributor

@Dimon4eg Dimon4eg commented May 7, 2021

This sets folders for targets to be visible in an IDE.

Before in VS (if tests ore on):
NC_3 2

Now in VS:
NC_3 2_folders

mgallien
mgallien requested changes May 7, 2021
View reviewed changes
Copy link
Collaborator

@mgallien mgallien left a comment

Choose a reason for hiding this comment

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

Thanks, nice idea
Please address my comment and we should good to go

@Dimon4eg Dimon4eg force-pushed the use_folders_in_cmake branch from 1c9fbcb to 041e6b9 Compare May 7, 2021 13:49
@Dimon4eg
Copy link
Contributor Author

@mgallien take a look again plz

@mgallien
Copy link
Collaborator

/rebase

@mgallien
Copy link
Collaborator

Thanks
Please do not merge master when working on a PR but rebase on top of master. We will always rebase before merging a PR and the history should not include merge like this one. I have to write some documentation about that to make contributing easier.

@Dimon4eg
Copy link
Contributor Author

I see, I just pressed button Update branch in this PR

@Dimon4eg Dimon4eg force-pushed the use_folders_in_cmake branch from 9f7c22b to 6d8dbbf Compare May 14, 2021 12:59
@allexzander
Copy link
Contributor

I see, I just pressed button Update branch in this PR

No worries. For the future, you will need to add a comment:

/rebase

in the PR. This will do the trick.

@github-actions github-actions bot force-pushed the use_folders_in_cmake branch from 6d8dbbf to 18d13a9 Compare May 17, 2021 06:32
@Dimon4eg Dimon4eg force-pushed the use_folders_in_cmake branch from dc1bf05 to b693821 Compare May 17, 2021 23:51
@Dimon4eg
Copy link
Contributor Author

use folders for nextcloudsync source files commit makes this:

image

@Dimon4eg Dimon4eg force-pushed the use_folders_in_cmake branch from b693821 to 951ef7b Compare May 18, 2021 00:32
@Dimon4eg
Copy link
Contributor Author

use folders for nextcloudCore source files commit makes this:

image

@allexzander allexzander self-requested a review May 18, 2021 06:38
@mgallien mgallien self-requested a review May 18, 2021 06:45
mgallien
mgallien requested changes May 18, 2021
View reviewed changes
Copy link
Collaborator

@mgallien mgallien left a comment

Choose a reason for hiding this comment

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

Please next time, once a PR is approved do not push more changes to it. That is not fair for the reviewers.

Please remove your new changes and push them to another PR. It would be nice if we can merge your first one.

@Dimon4eg
use folders in CMake
8b6b302 
Signed-off-by: Dmytro Korchynskyi <[email protected]>
@Dimon4eg Dimon4eg force-pushed the use_folders_in_cmake branch from 951ef7b to 8b6b302 Compare May 18, 2021 07:01
@Dimon4eg
Copy link
Contributor Author

Ok, sorry, no problem

@nextcloud-desktop-bot
Copy link

AppImage file: Nextcloud-PR-3286-8b6b3023eaaea9b7c6ab460851023195cab76870-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@mgallien mgallien self-requested a review May 18, 2021 10:16
@mgallien mgallien merged commit 701e106 into nextcloud:master May 18, 2021
@Dimon4eg Dimon4eg deleted the use_folders_in_cmake branch May 18, 2021 10:30
@Dimon4eg Dimon4eg mentioned this pull request May 18, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgallien mgallien mgallien approved these changes

@FlexW FlexW Awaiting requested review from FlexW

@camilasan camilasan Awaiting requested review from camilasan

@allexzander allexzander Awaiting requested review from allexzander

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Dimon4eg @mgallien @allexzander @nextcloud-desktop-bot