You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by "doupache (via GitHub)" <gi...@apache.org> on 2024/04/19 12:19:44 UTC

[PR] [YUNIKORN-2571] Add hierarchy icon to queue node [yunikorn-web]

doupache opened a new pull request, #189:
URL: https://github.com/apache/yunikorn-web/pull/189

   ### What is this PR for?
   Add hierarchy icon to queue node
   icon from [svgrepo](https://www.svgrepo.com/svg/497190/hierarchy) which is MIT License
   
   <img width="1299" alt="image" src="https://github.com/apache/yunikorn-web/assets/143783826/f1ebb569-6eaf-4080-83cf-8f5ee9c4b6f5">
   
   
   ### What type of PR is it?
   * [ ] - Improvement
   
   
   ### What is the Jira issue?
   [YUNIKORN-2571](https://issues.apache.org/jira/browse/YUNIKORN-2571)
   
   ### Screenshots (if appropriate)
   
   <img width="1156" alt="image" src="https://github.com/apache/yunikorn-web/assets/143783826/8516e524-a946-4125-8f9f-1bbdf90c46b4">
   


-- 
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: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [YUNIKORN-2571] Add hierarchy icon to queue node [yunikorn-web]

Posted by "wilfred-s (via GitHub)" <gi...@apache.org>.
wilfred-s commented on PR #189:
URL: https://github.com/apache/yunikorn-web/pull/189#issuecomment-2068450697

   We need to add a couple of lines to the License file. Like we did in the k8shim LICENSE file:
   ```
   --------------------------------------------------------------------------------
   This product bundles various third-party components under other open source
   licenses. This section summarizes those components and their licenses.
   
   MIT License
   ------------
   src/assets/images/hierarchy.svg
   ```
   
   Besides that the repo has the following comment:
   https://www.svgrepo.com/page/licensing/
   So we can add a text to the about box which we already need for our 3rd party licenses to make them easily accessible. This was doen in YUNIKORN-2448 so we should be able to piggy-back on that one


-- 
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: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [YUNIKORN-2571] Add hierarchy icon to queue node [yunikorn-web]

Posted by "chenyulin0719 (via GitHub)" <gi...@apache.org>.
chenyulin0719 commented on PR #189:
URL: https://github.com/apache/yunikorn-web/pull/189#issuecomment-2068321484

   Hi @wilfred-s, @craigcondit,
   
   What else we need to do to include the image from MIT license? 
   
   The [MIT lincese](https://opensource.org/license/mit) says "The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software."
   


-- 
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: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [YUNIKORN-2571] Add hierarchy icon to queue node [yunikorn-web]

Posted by "baconYao (via GitHub)" <gi...@apache.org>.
baconYao commented on code in PR #189:
URL: https://github.com/apache/yunikorn-web/pull/189#discussion_r1572437885


##########
src/app/components/queue-v2/queues-v2.component.ts:
##########
@@ -159,6 +159,13 @@ function queueVisualization(rawData : QueueInfo){
           .attr("height", 30)
           .attr("fill", "#e6f4ea")
           .attr("class", "cardBottom");
+        
+        group.append("image")
+        .attr("href", "./assets/images/hierarchy.svg") 
+        .attr("x", 5) 
+        .attr("y", 5) 
+        .attr("width", 20)
+        .attr("height", 20);

Review Comment:
   ```suggestion
           group.append("image")
             .attr("href", "./assets/images/hierarchy.svg") 
             .attr("x", 5) 
             .attr("y", 5) 
             .attr("width", 20)
             .attr("height", 20);
   ```



##########
src/app/components/queue-v2/queues-v2.component.ts:
##########
@@ -159,6 +159,13 @@ function queueVisualization(rawData : QueueInfo){
           .attr("height", 30)
           .attr("fill", "#e6f4ea")
           .attr("class", "cardBottom");
+        
+        group.append("image")
+        .attr("href", "./assets/images/hierarchy.svg") 
+        .attr("x", 5) 
+        .attr("y", 5) 
+        .attr("width", 20)
+        .attr("height", 20);

Review Comment:
   Should we care about the indent like other group here?



-- 
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: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [YUNIKORN-2571] Add hierarchy icon to queue node [yunikorn-web]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #189:
URL: https://github.com/apache/yunikorn-web/pull/189#issuecomment-2068305264

   ## [Codecov](https://app.codecov.io/gh/apache/yunikorn-web/pull/189?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   All modified and coverable lines are covered by tests :white_check_mark:
   > Project coverage is 38.77%. Comparing base [(`e4bc274`)](https://app.codecov.io/gh/apache/yunikorn-web/commit/e4bc27436aed022005d30581343c8a1085e8392f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`8e7d53c`)](https://app.codecov.io/gh/apache/yunikorn-web/pull/189?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   > Report is 1 commits behind head on master.
   
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@           Coverage Diff           @@
   ##           master     #189   +/-   ##
   =======================================
     Coverage   38.77%   38.77%           
   =======================================
     Files           2        2           
     Lines          49       49           
   =======================================
     Hits           19       19           
     Misses         27       27           
     Partials        3        3           
   ```
   
   
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/yunikorn-web/pull/189?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


-- 
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: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [YUNIKORN-2571] Add hierarchy icon to queue node [yunikorn-web]

Posted by "doupache (via GitHub)" <gi...@apache.org>.
doupache commented on code in PR #189:
URL: https://github.com/apache/yunikorn-web/pull/189#discussion_r1572506309


##########
src/app/components/queue-v2/queues-v2.component.ts:
##########
@@ -159,6 +159,13 @@ function queueVisualization(rawData : QueueInfo){
           .attr("height", 30)
           .attr("fill", "#e6f4ea")
           .attr("class", "cardBottom");
+        
+        group.append("image")
+        .attr("href", "./assets/images/hierarchy.svg") 
+        .attr("x", 5) 
+        .attr("y", 5) 
+        .attr("width", 20)
+        .attr("height", 20);

Review Comment:
   Thanks @baconYao.



-- 
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: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [YUNIKORN-2571] Add hierarchy icon to queue node [yunikorn-web]

Posted by "ryankert01 (via GitHub)" <gi...@apache.org>.
ryankert01 commented on PR #189:
URL: https://github.com/apache/yunikorn-web/pull/189#issuecomment-2067322278

   +1, Nice icon.


-- 
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: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org