Skip to content

create issue via github action for stale deps #16705#17876

Merged
phlax merged 26 commits intoenvoyproxy:mainfrom
ME-ON1:deps-action
Sep 29, 2021
Merged

create issue via github action for stale deps #16705#17876
phlax merged 26 commits intoenvoyproxy:mainfrom
ME-ON1:deps-action

Conversation

@ME-ON1
Copy link
Copy Markdown
Contributor

@ME-ON1 ME-ON1 commented Aug 26, 2021

Commit Message: added action to take a closer look of deps release
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

ME-ON1 added 6 commits August 26, 2021 23:04
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
@repokitteh-read-only
Copy link
Copy Markdown

Hi @ME-ON1, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #17876 was opened by ME-ON1.

see: more, trace.

ME-ON1 added 5 commits August 27, 2021 00:41
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
@phlax phlax self-assigned this Aug 27, 2021
@phlax
Copy link
Copy Markdown
Member

phlax commented Aug 27, 2021

cc @asraa

Copy link
Copy Markdown
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

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

Very nice start! Thank you so much for starting this

ME-ON1 added 3 commits August 27, 2021 23:49
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
@ME-ON1 ME-ON1 requested a review from asraa August 28, 2021 05:03
Copy link
Copy Markdown
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

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

line 186
if len(sys.argv) != 2:

might fail with this additional argument. you may want to update this.

i would also recommend doing the argument parsing in the main function, and add a boolean argument to verify_and_print_release_dates like create_issue given the flag presence (so you don't have to do arg parsing in the function)

Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
@ME-ON1 ME-ON1 requested a review from asraa September 2, 2021 07:32
@zuercher
Copy link
Copy Markdown
Member

zuercher commented Sep 8, 2021

ping @asraa

Copy link
Copy Markdown
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

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

Thanks! Did you try giving this a local run and just printing the issues created?

@ME-ON1
Copy link
Copy Markdown
Contributor Author

ME-ON1 commented Sep 9, 2021

Thanks! Did you try giving this a local run and just printing the issues created?

Yes tried locally with permissions and is running fine and created 3 issue also (by mistake :/ ) but without Label.

Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
@ME-ON1
Copy link
Copy Markdown
Contributor Author

ME-ON1 commented Sep 13, 2021

ping @asraa This one is working!

Copy link
Copy Markdown
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

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

other than the default cron arg lgtm

asraa
asraa previously approved these changes Sep 20, 2021
Copy link
Copy Markdown
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

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

LGTM! I was wrong about the default value

@alyssawilk
Copy link
Copy Markdown
Contributor

cc @phlax any thoughts or LGTY?

@phlax
Copy link
Copy Markdown
Member

phlax commented Sep 23, 2021

im happy to look over this PR but im OOO most of today

Copy link
Copy Markdown
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

this is great, thanks @ME-ON1

comments inline...

Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Copy link
Copy Markdown
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

@ME-ON1 this is really nearly ready to land

if we can clear up these last couple of nits, im happy to do so

thanks again for this

@ME-ON1
Copy link
Copy Markdown
Contributor Author

ME-ON1 commented Sep 28, 2021

@ME-ON1 this is really nearly ready to land

if we can clear up these last couple of nits, im happy to do so

thanks again for this

Im on it, to correct it ∠(^ー^)

Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Signed-off-by: Tarun Sharma <starun.1998@gmail.com>
Copy link
Copy Markdown
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @ME-ON1 for working through this

@ME-ON1
Copy link
Copy Markdown
Contributor Author

ME-ON1 commented Sep 29, 2021

Thanks for your patience too!! \o/

@phlax
Copy link
Copy Markdown
Member

phlax commented Sep 30, 2021

@ME-ON1 PR has landed and issues have been created - nice work

im thinking about a follow up PR with some cleanups/additions - in particular

  • Make the upstream name in the title literal with backticks - ie

...available rules_proto: 4.0.0

  • Add the current version at the end of the title - ie:

...rules_proto: 4.0.0 (current: 3.x)

  • improve formatting in the issue comment - provide separate lines with
    • package name
    • current version, date, info
    • available version, date, info
    • link to upstream repo

im also wondering about closing issues (similar to how dependabot works) such that if a newer update becomes available, any existing issues are closed, and also if the package is updated the issue should probs automatically close (if it wasnt closed by the PR to update)

if that is something you can/want to take on i would be happy to review, otherwise im happy to follow up

@phlax
Copy link
Copy Markdown
Member

phlax commented Sep 30, 2021

also, im a bit confused by the messaging here #18344

it says that there is a newer version (0.4.0) in the title and then clicking through it says there is a newer version than 0.4.0

i think it is because there is an actual release version, whereas we are using a repo/commit version - ie i think the issue is correct, just that the messaging is a bit confusing

@ME-ON1
Copy link
Copy Markdown
Contributor Author

ME-ON1 commented Sep 30, 2021

Make the upstream name in the title literal with back ticks.

I have a doubt if issue title in GitHub supports markdown formatting/code formatting

Add the current version at the end of the title - ie:

will do

improve formatting in the issue comment - provide separate lines with

will do

it says that there is a newer version (0.4.0) in the title and then clicking through it says there is a newer version than 0.4.0

Yeah this was confusing. Copied it from the create issue. This can be changed tho with another update. With all information in the body of the issue.

im also wondering about closing issues (similar to how dependabot works) such that if a newer update becomes available, any existing issues are closed, and also if the package is updated the issue should probs automatically close (if it wasnt closed by the PR to update)

Just to be more verbose. You mean like deps bot which will take look at every PR with reference to this ( update deps ) which closes issue related to it if its merged and also check if there is any updated version available in the already opened issue then this should close it and make a new issue ?

@phlax
Copy link
Copy Markdown
Member

phlax commented Sep 30, 2021

I have a doubt if issue title in GitHub supports markdown formatting/code formatting

it does - at least as far as literals are concerned

You mean like deps bot which will take look at every PR

so given a situation where there is an existing issue created for a package - lets call it com_github_foo

  • com_github_foo is behind in Envoy (new version is 1.0.0, a ticket was previously created)
  • ticket was not immediately resolved (eg because it had conflicts/issues)
  • new version of com_github_foo is release (eg 1.0.1)
  • ticket about 1.0.0 should close
  • new ticket for version 1.0.1 should be opened

@ME-ON1
Copy link
Copy Markdown
Contributor Author

ME-ON1 commented Sep 30, 2021

it does - at least as far as literals are concerned

OK then will make a new PR which have all the issue title and body changes ?

so given a situation where there is an existing issue created for a package - lets call it com_github_foo

yeah got it. Will be good to work on :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants