Skip to content
This repository was archived by the owner on Dec 14, 2021. It is now read-only.

feature: add close tabs to the right, #822#875

Merged
egraether merged 5 commits intomasterfrom
unknown repository
Jan 4, 2020
Merged

feature: add close tabs to the right, #822#875
egraether merged 5 commits intomasterfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Jan 2, 2020

Please let me know if this is ok. If something is missing just tell me.
As far as i can see this works just fine.
I have no automated test for this. Is this required?

@mlangkabel
Copy link
Contributor

Thank you for handing in this pull request! The failed CI build is probably not your fault. I'm looking into that right now.

@ghost
Copy link
Author

ghost commented Jan 3, 2020

Did you get a chance to look into it?
Is there anything i could do to help?

@egraether
Copy link
Contributor

I'm looking into it now.

Copy link
Contributor

@egraether egraether left a comment

Choose a reason for hiding this comment

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

Implementation looks fine. Please fix the formatting and white spacing issues. Thanks for implementing!

#include <QtContextMenu.h>
#include <QtGraphicsView.h>
#include <logging.h>
#include <QContextMenuEvent>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please follow convention:
For Qt headers we don't add '.h' e.g. #include <QContextMenuEvent>
For own headers we use double quotations e.g. #include "QtContextMenu.h"
Qt headers are placed above own headers (see other files)


void legendClicked();


Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this change.


void QtTabBar::contextMenuEvent(QContextMenuEvent* event)
{
QtContextMenu menu(event, this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Your whole implementation uses the wrong indentation. Please change everything to tabs instead of spaces.

@ghost
Copy link
Author

ghost commented Jan 3, 2020

thanks for the feedback. will make the changes and then turn on to the next issue...

@mlangkabel
Copy link
Contributor

Yes, I managed to fix the CI. So once you made the changes @egraether requested, you can make a new commit and the CI will automatically re-run the checks on your pull request.

@ghost
Copy link
Author

ghost commented Jan 3, 2020

So, i hope this is it :)
Have been struggling with KDevelop. I guess i'll switch back to vim. Much less trouble...

@egraether egraether merged commit 2422e09 into CoatiSoftware:master Jan 4, 2020
@egraether
Copy link
Contributor

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants