You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/06/23 21:49:02 UTC
[GitHub] [maven-fluido-skin] stevecrox opened a new pull request, #39: Mskins 188 improve maven logo
stevecrox opened a new pull request, #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39
Attempt to add an improved logo, checking this appears to work in the various views.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] slawekjaranowski commented on pull request #39: Mskins 188 improve maven logo
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1165648324
Something is miss
<img width="308" alt="image" src="https://user-images.githubusercontent.com/3578921/175559803-735d2fa3-88f8-488c-9483-2041fe475c8b.png">
logo should be inside gray box
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] stevecrox commented on a diff in pull request #39: Mskins 188 improve maven logo
Posted by GitBox <gi...@apache.org>.
stevecrox commented on code in PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#discussion_r905917107
##########
pom.xml:
##########
@@ -160,6 +160,7 @@ under the License.
<artifactId>apache-rat-plugin</artifactId>
<configuration>
<excludes combine.children="append">
+ <exclude>src/main/resources/images/logos/maven-feather.svg</exclude>
Review Comment:
Indeed, I've redone the commit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] stevecrox commented on pull request #39: Mskins 188 improve maven logo
Posted by GitBox <gi...@apache.org>.
stevecrox commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1165597761
The reason for replacement is the original is quite low resolution, picking a higher resolution image means it will scale down for smaller systems while looking good on 4K monitors as well.
I could create a couple different resolution images and then write CSS to set an element background image on a span element which I make window width dependent but the rationale there is to speed dial up style connections from loading the page.
The element itself is within a span which means a delay in loading won't affect page loading and should get alt text which would be displayed if there is an issue loading the image.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] michael-o commented on pull request #39: [MSKINS-188] improve maven logo
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1166273464
> For the record, as a Maven committer (who is not currently active) I disagree with the approach to merging. We could have merged this through a manual squash merge while retaining the author metadata.
>
> To the author, the commit rework would mean that your commit is fully attributable to you as both committer and author which is something that would be nice to have but shouldn’t be a roadblock
Correct, we always retain authorship.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] slawekjaranowski commented on pull request #39: [MSKINS-188] improve maven logo
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1166265991
Simply we require linear commit on master branch - from master perspective final result is important - we needn't history of work on PR in master.
Another reason is to have easy possibility to revert only one commit not many, if will needed.
I hope we don't waste your and our time for such discussion - please simply respect our easy requirement.
We can also accept PR and squash during merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] michael-o commented on pull request #39: Mskins 188 improve maven logo
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1164917009
Thanks, will check!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] stevecrox commented on pull request #39: [MSKINS-188] improve maven logo
Posted by GitBox <gi...@apache.org>.
stevecrox commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1166253424
I don't see any reasoning in that statement and its phrased in an unnecessary way.
I've already explained sound reasons for squashing a commit. This project appears to be following some variation of [Feature Branch Workflow](https://www.atlassian.com/git/tutorials/comparing-workflows/feature-branch-workflow). The key requirement of that approach is to ensure main (master is not inclusive) branch is always buildable and releasable.
The only reasons a commit in a branch matter are for the reasons I've outlined.
I'm not going to squash it and I figure you have the following choices.
- Provide a solid engineering based reason for squashing
- Accept the PR
- Reject the PR
If you accept it, I'll happily keep plugin away at updating the skin to Bootstrap v5 with all the UI aspects the project appears to value (I'm not sure I agree with a number of them some but whatever..).
If you reject it, I'll just fork and rename the project and release it as Bootstrap v5.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] stevecrox commented on pull request #39: Mskins 188 improve maven logo
Posted by GitBox <gi...@apache.org>.
stevecrox commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1165509103
I left in maven-feather.png (and updated it to the modern version) because the Integration Tests reference it and the older buttons.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] stevecrox commented on a diff in pull request #39: Mskins 188 improve maven logo
Posted by GitBox <gi...@apache.org>.
stevecrox commented on code in PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#discussion_r906087874
##########
pom.xml:
##########
@@ -160,6 +160,8 @@ under the License.
<artifactId>apache-rat-plugin</artifactId>
<configuration>
<excludes combine.children="append">
+ <exclude>src/main/resources/images/logos/maven-feather.png</exclude>
+ <exclude>src/main/resources/images/logos/powered-by-apache.png</exclude>
Review Comment:
I do not, let me redo the commits
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] stevecrox commented on pull request #39: [MSKINS-188] improve maven logo
Posted by GitBox <gi...@apache.org>.
stevecrox commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1165904732
Should be fixed, I was going through each commit to make sure they were limited to strictly only stuff that is required.
"row-fluid pft that doesn't relate to spans" he said to himself despite knowing spans are how the grid system work in Bootstrap 2 and then not checking after removing before his submitted the PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] stevecrox commented on pull request #39: [MSKINS-188] improve maven logo
Posted by GitBox <gi...@apache.org>.
stevecrox commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1166243693
Why?
Squashing should be used on long standing branches where people have made a mess of the commit chain (e.g. committing changes and a second commit to revert, or merging main into the branch periodically rather than rebasing).
But this is a feature branch from main, the resulting merge will ensure a continuous single chain of commits appear in the git changelog.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] stevecrox closed pull request #39: [MSKINS-188] improve maven logo
Posted by GitBox <gi...@apache.org>.
stevecrox closed pull request #39: [MSKINS-188] improve maven logo
URL: https://github.com/apache/maven-fluido-skin/pull/39
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] michael-o commented on pull request #39: [MSKINS-188] improve maven logo
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1166244688
> Why?
>
> Squashing should be used on long standing branches where people have made a mess of the commit chain (e.g. committing changes and a second commit to revert, or merging main into the branch periodically rather than rebasing).
>
> But this is a feature branch from main, the resulting merge will ensure a continuous single chain of commits appear in the git changelog.
No intermediate mess on master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] slawekjaranowski commented on pull request #39: Mskins 188 improve maven logo
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1165564943
@stevecrox if you start your commit message with issue in square brackets should be linked with jira,
like for this changes should be: [MSKINS-188]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] michael-o commented on a diff in pull request #39: Mskins 188 improve maven logo
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#discussion_r905512283
##########
pom.xml:
##########
@@ -160,6 +160,7 @@ under the License.
<artifactId>apache-rat-plugin</artifactId>
<configuration>
<excludes combine.children="append">
+ <exclude>src/main/resources/images/logos/maven-feather.svg</exclude>
Review Comment:
You have added a PNG, but excluding a SVG?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] slawekjaranowski commented on pull request #39: [MSKINS-188] improve maven logo
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1165911283
> The reason for replacement is the original is quite low resolution, picking a higher resolution image means it will scale down for smaller systems while looking good on 4K monitors as well. I lack a 4k monitor so I've kept it simple and used the original image.
>
> I could create a couple different resolution images and then write CSS to set an element background image on a span element which I make window width dependent but the rationale there is to speed dial up style connections from loading the page.
>
> The element itself is within a span which means a delay in loading won't affect page loading and should get alt text which would be displayed if there is an issue loading the image. Hence the KISS
I have display with resolution 3072 × 1920 and powered by logo have effective resolution 116 x 116
So is scaled from 2450 × 2450 -> 116 x 116
I only think if we really need so big input image? I'm not expert on this subject.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] michael-o commented on a diff in pull request #39: [MSKINS-188] improve maven logo
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#discussion_r906661843
##########
src/main/resources/META-INF/maven/site.vm:
##########
@@ -222,14 +222,17 @@
#* *##if ( $searchEnabled )
#* *##googleSearch( false )
#* *##end
- <div id="poweredBy">
+ <div id="poweredBy" class="row-fluid" >
#* *##facebookLike( $sideBarEnabled )
<div class="clear"></div>
#* *##followTwitter( $sideBarEnabled )
<div class="clear"></div>
#* *##flattrBody( $sideBarEnabled )
<div class="clear"></div>
-#* *##builtByLogo( $decoration.poweredBy )
+ <!-- MSKINS-188, added to centralise the logo. -->
Review Comment:
Drop this line, we have commit messages
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] slawekjaranowski commented on pull request #39: Mskins 188 improve maven logo
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1165570710
Do we need image `powered-by-apache.png` in original resource - `2450 × 2450 pixels`, file size is about `540 KB`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] stevecrox commented on pull request #39: [MSKINS-188] improve maven logo
Posted by GitBox <gi...@apache.org>.
stevecrox commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1166267183
I'm sorry picking one git hash value to roll back to over another is the same effort.
You've wasted me time, do it yourself I'm done with being insulted by the Maven team.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] slawekjaranowski commented on pull request #39: [MSKINS-188] improve maven logo
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1166243030
Finally please squash to one commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] stephenc commented on pull request #39: [MSKINS-188] improve maven logo
Posted by GitBox <gi...@apache.org>.
stephenc commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1166272334
For the record, as a Maven committer (who is not currently active) I disagree with the approach to merging. We could have merged this through a manual squash merge while retaining the author metadata.
To the author, the commit rework would mean that your commit is fully attributable to you as both committer and author which is something that would be nice to have but shouldn’t be a roadblock
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] slawekjaranowski commented on a diff in pull request #39: Mskins 188 improve maven logo
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#discussion_r906053853
##########
pom.xml:
##########
@@ -160,6 +160,8 @@ under the License.
<artifactId>apache-rat-plugin</artifactId>
<configuration>
<excludes combine.children="append">
+ <exclude>src/main/resources/images/logos/maven-feather.png</exclude>
+ <exclude>src/main/resources/images/logos/powered-by-apache.png</exclude>
Review Comment:
Do you need exclude binary file?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] stevecrox commented on pull request #39: [MSKINS-188] improve maven logo
Posted by GitBox <gi...@apache.org>.
stevecrox commented on PR #39:
URL: https://github.com/apache/maven-fluido-skin/pull/39#issuecomment-1166242124
So .. knowing its only 116 x 116 on a 4k screen gave me a target.
One of the factors I've included is accessibility I have worked with several people with sight issues and a common seems to be a 48" monitor combined with a few different ways of adding zoom on top. While working on the bootstrap upgrades I've been checking layout by reducing it to <578px width (for phones) and also leaving a normal layout but zoomed to 400% (it results in a display size similar to my co-workers setup).
- Using Gwenview reducing it to 5% (122 x 122) cut the size down to 14Kib, then zooming into 400% it looked like a blocky mess.
- Using Gwenview reducing it to 10% (245 x 245) cut the size down to 26Kib, then zooming into 400% it starts getting blocky but looks ok.
- Using Gwenview reducing it to 15% (367 x 367) cut the size down to 44Kib, then zooming into 400% it looks pretty good but is larger than my screen
I did a quick build of the Integration Tests and in v2.8 the side bar takes the entire screen width so checking with Gwenview was pretty accurate.
Is there any chance you could get the image size at a 400% zoom? I'm inclined to go with the 10% sized image (which I've committed).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org