Skip to content
This repository has been archived by the owner on May 19, 2024. It is now read-only.

Not working as expected when commenting multiple changed hunks in file #14

Closed
ahmetrehaseker opened this issue Feb 23, 2023 · 10 comments · Fixed by #15
Closed

Not working as expected when commenting multiple changed hunks in file #14

ahmetrehaseker opened this issue Feb 23, 2023 · 10 comments · Fixed by #15

Comments

@ahmetrehaseker
Copy link
Contributor

Describe the bug
If there is a one part is changed in file and you try to add a comment there it works but if there is a multiple changes hunks and you try to add a comment in second hunk library says it is not in pr.

To Reproduce
Steps to reproduce the behavior:

  1. Change first line and last line of a file and create a pr with that change
  2. Try to add a comment on last line
  3. Get the error CommentNotValidError

Expected behavior
Library should create the comment on the last line

@rym-dd
Copy link

rym-dd commented May 30, 2023

Hi @owenrumney

Do you think you can take a look to this issue?

Thanks.

@owenrumney
Copy link
Owner

I'm away till Sunday but have made a note to take a look then.

I think I understand the scenario so will try and reproduce then fix

@ahmetrehaseker
Copy link
Contributor Author

I have a small fix for this but unable to test it, I will create the pr today

@kobylianskii
Copy link

Hey @owenrumney! Do you think we can help somehow with this? There's an issue and I think many people are waiting for the fix.

@GregUK
Copy link

GregUK commented Nov 7, 2023

I have been trying to make use of the feature and I have hit this issue when attempting to use the tf-sec-commentor action

@owenrumney
Copy link
Owner

I'll take a look at the fix that was PR'd shortly.

It's my understanding that aqua are not long support tfsec or any of the associated tooling like the PR commenter. I think even when this is resolved here, it won't make it's way to tfsec commenter - Just fyi

@owenrumney
Copy link
Owner

released as v0.1.3 - good luck with getting it into tfsec

@GregUK
Copy link

GregUK commented Nov 9, 2023

Thanks @owenrumney, appreciate it.

@kobylianskii
Copy link

@owenrumney thanks a lot!

I think even when this is resolved here, it won't make it's way to tfsec commenter

But you're the main contributor there or am I wrong? 😑

@owenrumney
Copy link
Owner

@owenrumney thanks a lot!

I think even when this is resolved here, it won't make it's way to tfsec commenter

But you're the main contributor there or am I wrong? 😑

I was, till I left Aqua and lost my access

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 a pull request may close this issue.

5 participants