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

Imagebox resize svg fix #3881

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Elv13
Copy link
Member

@Elv13 Elv13 commented Dec 31, 2023

Fork of #3802 because I can't push to that branch.

Changes:

  • Fix luacheck
  • Remove the commit which rename the project

Copy link

codecov bot commented Dec 31, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cb72c0a) 91.01% compared to head (a44bd32) 91.01%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3881   +/-   ##
=======================================
  Coverage   91.01%   91.01%           
=======================================
  Files         901      901           
  Lines       57566    57574    +8     
=======================================
+ Hits        52392    52400    +8     
  Misses       5174     5174           
Flag Coverage Δ
gcov 91.01% <100.00%> (+<0.01%) ⬆️
luacov 93.72% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
lib/wibox/widget/imagebox.lua 96.55% <100.00%> (+0.12%) ⬆️

... and 2 files with indirect coverage changes

@Elv13 Elv13 closed this Dec 31, 2023
@Elv13 Elv13 reopened this Dec 31, 2023
@actionless
Copy link
Member

This branch has conflicts that must be resolved

@sclu1034
Copy link
Contributor

sclu1034 commented Jan 4, 2024

Fork of #3802 because I can't push to that branch.

You could have just asked them to uncheck the box, or tell them about the changes you'd like to see. Until merged, it's still their code, and the default for that setting is to allow for maintainers to push, so they consciously unchecked it.
So it doesn't look all too great for you to just disregard all of it, and make your own PR out of it, when you didn't even attempt to contact them.

And you wouldn't have to wait long for at least a comment. Crylia is still active in the Discord.

@actionless
Copy link
Member

Crylia is still active in the Discord.

the idea of hunting down someone on other platform in order to force them to finish their PR on github - is a bit on the edge of stalking 🤗

@sclu1034
Copy link
Contributor

sclu1034 commented Jan 4, 2024

What I meant to say with that is: If Elv had commented on the PR on GitHub, chances would have been very high that he would have gotten response.
And Elv is a member of the Discord server, so no stalking required.

@Crylia
Copy link
Contributor

Crylia commented Jan 8, 2024

Fork of #3802 because I can't push to that branch.

You could have just asked them to uncheck the box, or tell them about the changes you'd like to see. Until merged, it's still their code, and the default for that setting is to allow for maintainers to push, so they consciously unchecked it. So it doesn't look all too great for you to just disregard all of it, and make your own PR out of it, when you didn't even attempt to contact them.

And you wouldn't have to wait long for at least a comment. Crylia is still active in the Discord.

I didn't realize that button existed, and I checked it now. As for which one gets merged, I don't care too much, as the ultimate goal was to help fix some stuff (regardless of how much time I have to work on anything currently). So just pick the path of least resistance.

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.

4 participants