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

Clusterpedia robot for clusterpedia repository #231

Closed
wuyingjun-lucky opened this issue Jun 15, 2022 · 23 comments · Fixed by #294
Closed

Clusterpedia robot for clusterpedia repository #231

wuyingjun-lucky opened this issue Jun 15, 2022 · 23 comments · Fixed by #294
Assignees
Labels
kind/feature New feature

Comments

@wuyingjun-lucky
Copy link
Member

What would you like to be added?

Add clusterpedia robot to help to review code and pull request

Why is this needed?

Maintainer will merge the pull request after the pipeline succeed manually although maintainers and reviewers review and approve some small pull request at very early time.
I think we can introduce robot for improving efficiency in code review and merge request

@wuyingjun-lucky wuyingjun-lucky added the kind/feature New feature label Jun 15, 2022
@wuyingjun-lucky
Copy link
Member Author

@Iceber

@Iceber
Copy link
Member

Iceber commented Jun 15, 2022

Yes, it is necessary. My plan is to use https://github.com/wzshiming/gh-ci-bot to implement a simple robot, what do you think?

This is a example repo: https://github.com/wzshiming/gh-ci-bot-playground

@Iceber
Copy link
Member

Iceber commented Jun 15, 2022

But I'm a little worried about security, because we'll be handing over a very privileged token to the action to do something.

@wuyingjun-lucky
Copy link
Member Author

https://github.com/wzshiming/gh-ci-bot

It seems lack of lgtm command.

@wuyingjun-lucky
Copy link
Member Author

But I'm a little worried about security, because we'll be handing over a very privileged token to the action to do something.

In other words . The maintainers and reviewers invited in the future can not write the repository directly. Maybe it is more safe .

@Iceber
Copy link
Member

Iceber commented Jun 15, 2022

@wzshiming https://github.com/wzshiming/gh-ci-bot is missing a common command lgtm —— add an lgtm lable.

Although /label-lgtm can also be used, it seems that /lgtm is more commonly used.

@Iceber
Copy link
Member

Iceber commented Jun 15, 2022

The maintainers and reviewers invited in the future can not write the repository directly. Maybe it is more safe .

Yes, we can consider doing this.

There is a security issue, the gh-ci-botscripts need to be pulled from the https://github.com/wzshiming/gh-ci-bot to be executed (detail), and this can cause more serious problems if scripts in the repository are replaced with malicious scripts due to some problems.

Can we fork to clusterpedia-io, which is a repository that only the maintainer has write access to?

@wuyingjun-lucky
Copy link
Member Author

clusterpedia

Sounds good.
Actually, In our personal gitlab repo, we also have a project named gitlab-bot in our private group to do this.

@Iceber
Copy link
Member

Iceber commented Jun 15, 2022

Your gitlab-bot seems to work fine, would you like to migrate/contribute to clusterpedia-io? We can simply compare the two tools and choose the easiest and most convenient one.

@wuyingjun-lucky
Copy link
Member Author

Your gitlab-bot seems to work fine, would you like to migrate/contribute to clusterpedia-io? We can simply compare the two tools and choose the easiest and most convenient one.

I'd like to
But the gitlab open api is different with github
and
our bot is looks too simply to satisfy clusterpedia's needs
I will try to find some way in my spare time.

@Iceber
Copy link
Member

Iceber commented Jun 15, 2022

I will try to find some way in my spare time.

This issue is not particularly pressing and we can have more discussions 😄

@wzshiming
Copy link
Member

@wzshiming https://github.com/wzshiming/gh-ci-bot is missing a common command lgtm —— add an lgtm lable.

Although /label-lgtm can also be used, it seems that /lgtm is more commonly used.

https://github.com/wzshiming/gh-ci-bot/tree/master/plugins/label-lgtm

/lgtm and /remove-lgtm

It is already supported, because lgtm is not the default labels on Github and needs to be created in the project in advance.

@Iceber
Copy link
Member

Iceber commented Jun 15, 2022

@wzshiming https://github.com/wzshiming/gh-ci-bot#gh-ci-bot looks like it needs to be updated:joy:.

@wzshiming
Copy link
Member

Updated 😃

@wuyingjun-lucky
Copy link
Member Author

ping @Iceber @wzshiming when to apply the bot to clusterpedia

@Iceber
Copy link
Member

Iceber commented Aug 5, 2022

@wuyingjun-lucky Thank you for the follow up, We will soon add robot.

@wzshiming will use https://github.com/wzshiming/gh-ci-bot to support the robot, thanks to the help of @wzshiming 🌹

@Iceber
Copy link
Member

Iceber commented Aug 5, 2022

There is a security issue, the gh-ci-botscripts need to be pulled from the https://github.com/wzshiming/gh-ci-bot to be executed (detail), and this can cause more serious problems if scripts in the repository are replaced with malicious scripts due to some problems.

Can we fork to clusterpedia-io, which is a repository that only the maintainer has write access to?

@wzshiming gh-ci-bot has been forked to https://github.com/clusterpedia-io/gh-ci-bot.

@wzshiming wzshiming mentioned this issue Aug 7, 2022
@Iceber
Copy link
Member

Iceber commented Aug 8, 2022

#294 adds commands for lable and cc, assign,etc...

We currently do not use automatic merging due to the small number of approvers.

@wuyingjun-lucky
Copy link
Member Author

#294 adds commands for lable and cc, assign,etc...

We currently do not use automatic merging due to the small number of approvers.

Why not ?
If I just modify an error words. I think you do not need to merge it until the pipeline succeed. It can save your time.

@Iceber
Copy link
Member

Iceber commented Aug 8, 2022

Why not ?
If I just modify an error words. I think you do not need to merge it until the pipeline succeed. It can save your time.

Auto-merge is good for multiple reviewers and approvers to work together, but right now I'm the only approver, and the gh-ci-bot doesn't support auto-merge yet 😂

@wuyingjun-lucky
Copy link
Member Author

Why not ?
If I just modify an error words. I think you do not need to merge it until the pipeline succeed. It can save your time.

Auto-merge is good for multiple reviewers and approvers to work together, but right now I'm the only approver, and the gh-ci-bot doesn't support auto-merge yet 😂

All right. Label merge can not auto-merge. I think auto-merge can save your time.

@wzshiming
Copy link
Member

wzshiming/gh-ci-bot#14
I create an issue for this auto-merge, and I will take the time to do it later

@Iceber
Copy link
Member

Iceber commented Aug 8, 2022

Currently I am the only approver, so I will review a pr several times before merging it, currently it feels better for me to merge it manually.

In the future, when there are more approvers, we can switch to automatic merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants