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

Context deletes review comments from different context #30

Open
davidtvs opened this issue May 10, 2020 · 3 comments
Open

Context deletes review comments from different context #30

davidtvs opened this issue May 10, 2020 · 3 comments

Comments

@davidtvs
Copy link
Contributor

I've setup Lintly to run flake8 and black in my repository however after some tests I noticed that even though the two of them produce different statuses within a review I'll only see the review comments of the one that was last run.

So if I run flake8 first, I'll get flake8's review comments but then if I run black, all the comments from flake8 are deleted and I'll only see the ones from black.

I think a context should only delete comments that were created by that same context. Or, I would even prefer an option for nothing to be deleted, the comments are an important part of the review process.

@grantmcconnaughey
Copy link
Owner

Good catch! I hadn’t thought of this issue before. I think that should be an easy-ish fix. I use hidden HTML to identify which comments to delete. I’ll just need to add the context to that.

@davidtvs
Copy link
Contributor Author

Nice 👍. What do you think of having an option that doesn't delete any of the previous comments? Personally, I would prefer that.

@grantmcconnaughey
Copy link
Owner

I think that makes a lot of sense, too. Now that GitHub hides previous comments as "resolved" it's less important to clean those up. In fact that would probably make more sense as the default behavior.

renefritze pushed a commit to renefritze/Lintly that referenced this issue Feb 21, 2022
* Fixing f-string

In a previous commit broke builds.py:109 into two lines so that flake8 would stop complaining about line length. This introduced a bug where the second line was not an f-string. This PR makes both lines f-strings.

* Bumping version to 0.7.3

* Using python's implied line continuation
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

No branches or pull requests

2 participants