Skip to content

  • Projects
  • Groups
  • Snippets
  • Help
  • This project
    • Loading...
  • Sign in / Register
GitLab Enterprise Edition
GitLab Enterprise Edition
  • Overview
    • Overview
    • Details
    • Activity
    • Cycle Analytics
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Charts
    • Locked Files
  • Issues 2,413
    • Issues 2,413
    • List
    • Boards
    • Labels
    • Service Desk
    • Milestones
  • Merge Requests 199
    • Merge Requests 199
  • CI / CD
    • CI / CD
    • Pipelines
    • Jobs
    • Schedules
    • Charts
  • Snippets
    • Snippets
  • Members
    • Members
  • Collapse sidebar
  • Activity
  • Graph
  • Charts
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
  • GitLab.org
  • GitLab Enterprise EditionGitLab Enterprise Edition
  • Merge Requests
  • !1540

Merged
Opened Mar 29, 2017 by Mike Greiling@mikegreiling 13 of 13 tasks completed13/13 tasks
  • Report abuse
Report abuse

Burndown Charts

Screen Shots

Help box Milestone in progress
help-box burndown-chart

Tooltips/Animation/Sidebar in action:

burndown-chart

Instructions for reviewer:

Seed burndown chart data with the following command:

rake db:seed_fu FILTER=burndown

You should then see a milestone starting with something like "Sprint - lorem ipsum ...". This should contain enough issues with weight to demonstrate the milestone chart. You can adjust the milestone start and due date to see how it would show an in-progress milestone.

To Do (frontend):

  • create burndown chart empty state when no start/due date defined
  • implement basic chart layout in d3
  • add "ideal" line to graph
  • add "actual" line using test data sample
  • make chart responsive to window resize
  • make chart responsive to animated right sidebar open/close toggle
  • add header
  • add chart legend
  • add y-axis label
  • update to use database-fed data instead of hard coded test data (once BE component is finished)
  • code optimizations

Stretch:

  • add tooltips
  • add toggle to view issue weight

What are the relevant issue numbers?

Closes #91 (closed)

  • Discussion 102
  • Commits 48
  • Pipelines 39
  • Changes 14
{{ resolvedDiscussionCount }}/{{ discussionCount }} {{ resolvedCountText }} resolved
  • Mike Greiling @mikegreiling

    added 284 commits

    • f9fe4593...e34923aa - 272 commits from branch master
    • 101a3b66 - Merge branch '23674-simplify-milestone-summary-ee' into 91-milestone-burndown-charts
    • b3d1dd1e - create burndown chart template and js bundle
    • 974e2fa7 - implement burndown chart hint message
    • 89b80350 - move burndown chart before user action hints
    • 6e57e5a9 - implement burndown chart basic outline
    • bfe0a623 - correctly handle empty array passed to setData
    • ae6ce624 - render burndown chart lines
    • 9a877250 - allow burndown chart to resize during animations
    • 4a372b5a - defer render call to the next animation frame to avoid layout thrashing
    • 9c078713 - fix axis font size in burndown chart
    • 85e68eba - add overflow to the x-axis domain
    • 6b5fdb52 - create x-axis line in constructor instead of regenerating on each render

    Compare with previous version

    Mar 29, 2017

    added 284 commits

    • f9fe4593...e34923aa - 272 commits from branch master
    • 101a3b66 - Merge branch '23674-simplify-milestone-summary-ee' into 91-milestone-burndown-charts
    • b3d1dd1e - create burndown chart template and js bundle
    • 974e2fa7 - implement burndown chart hint message
    • 89b80350 - move burndown chart before user action hints
    • 6e57e5a9 - implement burndown chart basic outline
    • bfe0a623 - correctly handle empty array passed to setData
    • ae6ce624 - render burndown chart lines
    • 9a877250 - allow burndown chart to resize during animations
    • 4a372b5a - defer render call to the next animation frame to avoid layout thrashing
    • 9c078713 - fix axis font size in burndown chart
    • 85e68eba - add overflow to the x-axis domain
    • 6b5fdb52 - create x-axis line in constructor instead of regenerating on each render

    Compare with previous version

    added 284 commits * f9fe4593...e34923aa - 272 commits from branch `master` * 101a3b66 - Merge branch '23674-simplify-milestone-summary-ee' into 91-milestone-burndown-charts * b3d1dd1e - create burndown chart template and js bundle * 974e2fa7 - implement burndown chart hint message * 89b80350 - move burndown chart before user action hints * 6e57e5a9 - implement burndown chart basic outline * bfe0a623 - correctly handle empty array passed to setData * ae6ce624 - render burndown chart lines * 9a877250 - allow burndown chart to resize during animations * 4a372b5a - defer render call to the next animation frame to avoid layout thrashing * 9c078713 - fix axis font size in burndown chart * 85e68eba - add overflow to the x-axis domain * 6b5fdb52 - create x-axis line in constructor instead of regenerating on each render [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3641672&start_sha=f9fe4593ccefb17ceb2343839f7bb03113ce4962)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 15 commits

    • 6b5fdb52...2d9fbfdc - 4 commits from branch master
    • dfe504ea - create burndown chart template and js bundle
    • 43e08f9a - implement burndown chart hint message
    • 48cbca5c - move burndown chart before user action hints
    • 20247f10 - implement burndown chart basic outline
    • 51fd4c93 - correctly handle empty array passed to setData
    • 78571b3d - render burndown chart lines
    • 25007294 - allow burndown chart to resize during animations
    • 54df6a4d - defer render call to the next animation frame to avoid layout thrashing
    • 0df65beb - fix axis font size in burndown chart
    • c736d11d - add overflow to the x-axis domain
    • 42a8387a - create x-axis line in constructor instead of regenerating on each render

    Compare with previous version

    Mar 29, 2017

    added 15 commits

    • 6b5fdb52...2d9fbfdc - 4 commits from branch master
    • dfe504ea - create burndown chart template and js bundle
    • 43e08f9a - implement burndown chart hint message
    • 48cbca5c - move burndown chart before user action hints
    • 20247f10 - implement burndown chart basic outline
    • 51fd4c93 - correctly handle empty array passed to setData
    • 78571b3d - render burndown chart lines
    • 25007294 - allow burndown chart to resize during animations
    • 54df6a4d - defer render call to the next animation frame to avoid layout thrashing
    • 0df65beb - fix axis font size in burndown chart
    • c736d11d - add overflow to the x-axis domain
    • 42a8387a - create x-axis line in constructor instead of regenerating on each render

    Compare with previous version

    added 15 commits * 6b5fdb52...2d9fbfdc - 4 commits from branch `master` * dfe504ea - create burndown chart template and js bundle * 43e08f9a - implement burndown chart hint message * 48cbca5c - move burndown chart before user action hints * 20247f10 - implement burndown chart basic outline * 51fd4c93 - correctly handle empty array passed to setData * 78571b3d - render burndown chart lines * 25007294 - allow burndown chart to resize during animations * 54df6a4d - defer render call to the next animation frame to avoid layout thrashing * 0df65beb - fix axis font size in burndown chart * c736d11d - add overflow to the x-axis domain * 42a8387a - create x-axis line in constructor instead of regenerating on each render [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3642457&start_sha=6b5fdb520fc28de45b9ac4b905e8d9c00d24f591)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 1 commit

    • 73ea691d - add label to the y-axis

    Compare with previous version

    Mar 30, 2017

    added 1 commit

    • 73ea691d - add label to the y-axis

    Compare with previous version

    added 1 commit * 73ea691d - add label to the y-axis [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3644096&start_sha=42a8387ac6d9898886f96c39203d105cbdf96dfa)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 1 commit

    • 5e77568d - ideal line should end at dueDate not maxDate in event that milestone goes overdue

    Compare with previous version

    Mar 30, 2017

    added 1 commit

    • 5e77568d - ideal line should end at dueDate not maxDate in event that milestone goes overdue

    Compare with previous version

    added 1 commit * 5e77568d - ideal line should end at dueDate not maxDate in event that milestone goes overdue [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3644119&start_sha=73ea691dddb9a996f61aff6bf6b9f3b513f26881)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 2 commits

    • a4d995c0 - create burndown chart legend
    • 43efea41 - add header to burndown chart and adjust chart size breakpoints

    Compare with previous version

    Mar 30, 2017

    added 2 commits

    • a4d995c0 - create burndown chart legend
    • 43efea41 - add header to burndown chart and adjust chart size breakpoints

    Compare with previous version

    added 2 commits * a4d995c0 - create burndown chart legend * 43efea41 - add header to burndown chart and adjust chart size breakpoints [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3644969&start_sha=5e77568dd4308a729f8f7bf7d07987827e43c063)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 1 commit

    • cba3a5f7 - add 800ms animation to the line path whenever setData is called

    Compare with previous version

    Mar 30, 2017

    added 1 commit

    • cba3a5f7 - add 800ms animation to the line path whenever setData is called

    Compare with previous version

    added 1 commit * cba3a5f7 - add 800ms animation to the line path whenever setData is called [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3645830&start_sha=43efea41ce8182549766e60ab1db4f8498056212)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 1 commit

    • 47a5821c - implement first pass at tooltips feature

    Compare with previous version

    Mar 30, 2017

    added 1 commit

    • 47a5821c - implement first pass at tooltips feature

    Compare with previous version

    added 1 commit * 47a5821c - implement first pass at tooltips feature [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3647209&start_sha=cba3a5f7adbcacebc05d29618e919a642fd05424)
    Toggle commit list
  • Mike Greiling @mikegreiling

    marked the task add header as completed

    Mar 30, 2017

    marked the task add header as completed

    marked the task **add header** as completed
    Toggle commit list
  • Mike Greiling @mikegreiling

    marked the task add chart legend as completed

    Mar 30, 2017

    marked the task add chart legend as completed

    marked the task **add chart legend** as completed
    Toggle commit list
  • Mike Greiling @mikegreiling

    marked the task add y-axis label as completed

    Mar 30, 2017

    marked the task add y-axis label as completed

    marked the task **add y-axis label** as completed
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 1 commit

    • 9a894109 - improve tooltip design and placement

    Compare with previous version

    Mar 30, 2017

    added 1 commit

    • 9a894109 - improve tooltip design and placement

    Compare with previous version

    added 1 commit * 9a894109 - improve tooltip design and placement [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3659407&start_sha=47a5821c01bbffa6b1197035fa5116420106a8a9)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 1 commit

    • a53bd8c8 - improve tooltip design and placement

    Compare with previous version

    Mar 30, 2017

    added 1 commit

    • a53bd8c8 - improve tooltip design and placement

    Compare with previous version

    added 1 commit * a53bd8c8 - improve tooltip design and placement [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3659527&start_sha=9a894109e7756ad1122b12be072977b19eaab7fe)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 1 commit

    • 839aaa36 - hide tooltips during chart animation

    Compare with previous version

    Mar 30, 2017

    added 1 commit

    • 839aaa36 - hide tooltips during chart animation

    Compare with previous version

    added 1 commit * 839aaa36 - hide tooltips during chart animation [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3659673&start_sha=a53bd8c88b19724f1f960dfacad2de63c56609c1)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 2 commits

    • e827923e - rename resize and animate methods for clarity
    • 0578ee29 - remove posessives in comment blocks

    Compare with previous version

    Mar 31, 2017

    added 2 commits

    • e827923e - rename resize and animate methods for clarity
    • 0578ee29 - remove posessives in comment blocks

    Compare with previous version

    added 2 commits * e827923e - rename resize and animate methods for clarity * 0578ee29 - remove posessives in comment blocks [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3678826&start_sha=839aaa36977a321abfa169ddc8db975cda1be78d)
    Toggle commit list
  • Mike Greiling @mikegreiling

    marked the task add tooltips as completed

    Mar 31, 2017

    marked the task add tooltips as completed

    marked the task **add tooltips** as completed
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 3 commits

    • 4293a046 - rename and reorder additional methods for clarity
    • 4f187dbd - update tooltip label through setData options
    • 6f13b46d - fix phantomjs mishandling of svgs with implicit dimensions

    Compare with previous version

    Mar 31, 2017

    added 3 commits

    • 4293a046 - rename and reorder additional methods for clarity
    • 4f187dbd - update tooltip label through setData options
    • 6f13b46d - fix phantomjs mishandling of svgs with implicit dimensions

    Compare with previous version

    added 3 commits * 4293a046 - rename and reorder additional methods for clarity * 4f187dbd - update tooltip label through setData options * 6f13b46d - fix phantomjs mishandling of svgs with implicit dimensions [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3680060&start_sha=0578ee29ae15a3f356ba18ebedf9601cc70ccb0f)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 239 commits

    • 6f13b46d...cf91b76a - 215 commits from branch master
    • 78fd5405 - create burndown chart template and js bundle
    • 6e4361c4 - implement burndown chart hint message
    • b3a0db53 - move burndown chart before user action hints
    • bd08a573 - implement burndown chart basic outline
    • c6084c94 - correctly handle empty array passed to setData
    • 57e38c36 - render burndown chart lines
    • 88ced2e6 - allow burndown chart to resize during animations
    • 3940508a - defer render call to the next animation frame to avoid layout thrashing
    • 2feaf499 - fix axis font size in burndown chart
    • dc8e620c - add overflow to the x-axis domain
    • f1363e33 - create x-axis line in constructor instead of regenerating on each render
    • c3fd9c7e - add label to the y-axis
    • f01c25e0 - ideal line should end at dueDate not maxDate in event that milestone goes overdue
    • c7ade5b9 - create burndown chart legend
    • 8fbda9f9 - add header to burndown chart and adjust chart size breakpoints
    • ee9318d5 - add 800ms animation to the line path whenever setData is called
    • 3750a477 - implement first pass at tooltips feature
    • 733521b2 - improve tooltip design and placement
    • f8057eb9 - hide tooltips during chart animation
    • aab01927 - rename resize and animate methods for clarity
    • a19c99aa - remove posessives in comment blocks
    • 3b6478fd - rename and reorder additional methods for clarity
    • d9093464 - update tooltip label through setData options
    • 601142f9 - fix phantomjs mishandling of svgs with implicit dimensions

    Compare with previous version

    Mar 31, 2017

    added 239 commits

    • 6f13b46d...cf91b76a - 215 commits from branch master
    • 78fd5405 - create burndown chart template and js bundle
    • 6e4361c4 - implement burndown chart hint message
    • b3a0db53 - move burndown chart before user action hints
    • bd08a573 - implement burndown chart basic outline
    • c6084c94 - correctly handle empty array passed to setData
    • 57e38c36 - render burndown chart lines
    • 88ced2e6 - allow burndown chart to resize during animations
    • 3940508a - defer render call to the next animation frame to avoid layout thrashing
    • 2feaf499 - fix axis font size in burndown chart
    • dc8e620c - add overflow to the x-axis domain
    • f1363e33 - create x-axis line in constructor instead of regenerating on each render
    • c3fd9c7e - add label to the y-axis
    • f01c25e0 - ideal line should end at dueDate not maxDate in event that milestone goes overdue
    • c7ade5b9 - create burndown chart legend
    • 8fbda9f9 - add header to burndown chart and adjust chart size breakpoints
    • ee9318d5 - add 800ms animation to the line path whenever setData is called
    • 3750a477 - implement first pass at tooltips feature
    • 733521b2 - improve tooltip design and placement
    • f8057eb9 - hide tooltips during chart animation
    • aab01927 - rename resize and animate methods for clarity
    • a19c99aa - remove posessives in comment blocks
    • 3b6478fd - rename and reorder additional methods for clarity
    • d9093464 - update tooltip label through setData options
    • 601142f9 - fix phantomjs mishandling of svgs with implicit dimensions

    Compare with previous version

    added 239 commits * 6f13b46d...cf91b76a - 215 commits from branch `master` * 78fd5405 - create burndown chart template and js bundle * 6e4361c4 - implement burndown chart hint message * b3a0db53 - move burndown chart before user action hints * bd08a573 - implement burndown chart basic outline * c6084c94 - correctly handle empty array passed to setData * 57e38c36 - render burndown chart lines * 88ced2e6 - allow burndown chart to resize during animations * 3940508a - defer render call to the next animation frame to avoid layout thrashing * 2feaf499 - fix axis font size in burndown chart * dc8e620c - add overflow to the x-axis domain * f1363e33 - create x-axis line in constructor instead of regenerating on each render * c3fd9c7e - add label to the y-axis * f01c25e0 - ideal line should end at dueDate not maxDate in event that milestone goes overdue * c7ade5b9 - create burndown chart legend * 8fbda9f9 - add header to burndown chart and adjust chart size breakpoints * ee9318d5 - add 800ms animation to the line path whenever setData is called * 3750a477 - implement first pass at tooltips feature * 733521b2 - improve tooltip design and placement * f8057eb9 - hide tooltips during chart animation * aab01927 - rename resize and animate methods for clarity * a19c99aa - remove posessives in comment blocks * 3b6478fd - rename and reorder additional methods for clarity * d9093464 - update tooltip label through setData options * 601142f9 - fix phantomjs mishandling of svgs with implicit dimensions [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3680588&start_sha=6f13b46d719ede781b102bc065f308484f26f982)
    Toggle commit list
  • Felipe Artur
    @felipe_artur started a discussion on an old version of the diff Mar 31, 2017
    Resolved by Mike Greiling Mar 31, 2017
    app/assets/javascripts/burndown_chart/test_data.json 0 → 100644
    Unable to load the diff.
    • Felipe Artur @felipe_artur commented Mar 31, 2017
      Developer

      no weight on test data?

      no weight on test data?
    • Mike Greiling @mikegreiling commented Mar 31, 2017
      Master

      I'll add that in once we've merged the two branches. It wasn't needed to test the frontend chart code because it'll work the same in either case (just changes the labels).

      Edited Mar 31, 2017 by Mike Greiling
      I'll add that in once we've merged the two branches. It wasn't needed to test the frontend chart code because it'll work the same in either case (just changes the labels).
    Please register or sign in to reply
  • Mike Greiling @mikegreiling

    added 3 commits

    • 028be8b7 - Burndown Chart backend implementation
    • 226202e0 - Improve algorithm to avoid double queries on postgres and mysql
    • c82ac9bb - Merge branch 'issue_91' into '91-milestone-burndown-charts'

    Compare with previous version

    Mar 31, 2017

    added 3 commits

    • 028be8b7 - Burndown Chart backend implementation
    • 226202e0 - Improve algorithm to avoid double queries on postgres and mysql
    • c82ac9bb - Merge branch 'issue_91' into '91-milestone-burndown-charts'

    Compare with previous version

    added 3 commits * 028be8b7 - Burndown Chart backend implementation * 226202e0 - Improve algorithm to avoid double queries on postgres and mysql * c82ac9bb - Merge branch 'issue_91' into '91-milestone-burndown-charts' [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3681915&start_sha=601142f99b05b3192bbcbf8d44fc6695d8c49860)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 1 commit

    • 88475528 - remove test_data and rely on data passed from rails

    Compare with previous version

    Mar 31, 2017

    added 1 commit

    • 88475528 - remove test_data and rely on data passed from rails

    Compare with previous version

    added 1 commit * 88475528 - remove test_data and rely on data passed from rails [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3681968&start_sha=c82ac9bba42c027b0a66dbe6d86a018d78e5360f)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 1 commit

    • cd11348a - add toggle between issue count and issue weight

    Compare with previous version

    Mar 31, 2017

    added 1 commit

    • cd11348a - add toggle between issue count and issue weight

    Compare with previous version

    added 1 commit * cd11348a - add toggle between issue count and issue weight [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3682277&start_sha=88475528836e6e40289d2edb957790fe012f1a0b)
    Toggle commit list
  • Mike Greiling @mikegreiling

    marked the task update to use database-fed data instead of hard coded test data (once BE component is finished) as completed

    Mar 31, 2017

    marked the task update to use database-fed data instead of hard coded test data (once BE component is finished) as completed

    marked the task **update to use database-fed data instead of hard coded test data (once BE component is finished)** as completed
    Toggle commit list
  • Mike Greiling @mikegreiling

    marked the task add toggle to view issue weight as completed

    Mar 31, 2017

    marked the task add toggle to view issue weight as completed

    marked the task **add toggle to view issue weight** as completed
    Toggle commit list
  • Mike Greiling @mikegreiling

    resolved all discussions

    Mar 31, 2017

    resolved all discussions

    resolved all discussions
    Toggle commit list
  • Felipe Artur @felipe_artur

    added 1 commit

    • b1164e1a - Show chart only until current date when milestone due date is bigger

    Compare with previous version

    Mar 31, 2017

    added 1 commit

    • b1164e1a - Show chart only until current date when milestone due date is bigger

    Compare with previous version

    added 1 commit * b1164e1a - Show chart only until current date when milestone due date is bigger [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3682551&start_sha=cd11348aeccbfe0c8eecbe8d72b3ea24a3b23cf8)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 1 commit

    • 7e658aa6 - adjust burndown chart data toggle style

    Compare with previous version

    Mar 31, 2017

    added 1 commit

    • 7e658aa6 - adjust burndown chart data toggle style

    Compare with previous version

    added 1 commit * 7e658aa6 - adjust burndown chart data toggle style [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3682627&start_sha=b1164e1a89439fc56278e04e50d57be2d62a2828)
    Toggle commit list
  • Mike Greiling @mikegreiling

    unmarked as a Work In Progress

    Mar 31, 2017

    unmarked as a Work In Progress

    unmarked as a **Work In Progress**
    Toggle commit list
  • Mike Greiling @mikegreiling

    changed title from WIP Burndown charts (frontend) to Burndown Charts

    Mar 31, 2017

    changed title from WIP Burndown charts (frontend) to Burndown Charts

    changed title from **{-WIP Burndown charts (frontend)-}** to **{+Burndown Charts+}**
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 1 commit

    • 1864d0de - fix end date not matching due date

    Compare with previous version

    Mar 31, 2017

    added 1 commit

    • 1864d0de - fix end date not matching due date

    Compare with previous version

    added 1 commit * 1864d0de - fix end date not matching due date [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3682754&start_sha=7e658aa6dad5e0e7bea0933f092f410845de5795)
    Toggle commit list
  • Mike Greiling @mikegreiling

    assigned to @jschatz1

    Mar 31, 2017

    assigned to @jschatz1

    assigned to @jschatz1
    Toggle commit list
  • Jacob Schatz @jschatz1 commented Apr 01, 2017
    Master

    This GIF does not do justice. I scrolled down and got a seizure from some sort of scroll detection and I don't see a burndown chart. I prob don't see it because there isn't any data but you should check it out sans data.

    cooooldude

    This GIF does not do justice. I scrolled down and got a seizure from some sort of scroll detection and I don't see a burndown chart. I prob don't see it because there isn't any data but you should check it out sans data. ![cooooldude](/uploads/814d17c40a86d8b15f93c4eeffe89a81/cooooldude.gif)
  • Jacob Schatz @jschatz1 commented Apr 01, 2017
    Master

    @mikegreiling I had to take an actual video of my screen to prove it.

    VID_20170331_202134

    Edited Apr 01, 2017 by Jacob Schatz
    @mikegreiling I had to take an actual video of my screen to prove it. ![VID_20170331_202134](/uploads/4551060a4af58cca30ebf713d43b1950/VID_20170331_202134.mp4)
  • Jacob Schatz @jschatz1 commented Apr 01, 2017
    Master

    @mikegreiling Actually I just noticed the burndown doesn't show up unless you are signed in. And that jitter happens when you aren't signed it because the burndown chart isn't there.

    Edited Apr 01, 2017 by Jacob Schatz
    @mikegreiling Actually I just noticed the burndown doesn't show up unless you are signed in. And that jitter happens when you aren't signed it because the burndown chart isn't there.
  • Jacob Schatz @jschatz1 commented Apr 01, 2017
    Master

    I would just check a few things with:

    1. No data.
    2. No user logged in.

    Might have overlooked a few things.

    I would just check a few things with: 1. No data. 1. No user logged in. Might have overlooked a few things.
  • Mike Greiling @mikegreiling commented Apr 01, 2017
    Master

    @jschatz1 that bug is actually unrelated to the burndown chart. it's the sidebar logic. @annabeldunstone already merged a fix in CE, but it hasn't made its way to EE yet

    @jschatz1 that bug is actually unrelated to the burndown chart. it's the sidebar logic. @annabeldunstone already merged a fix in CE, but it hasn't made its way to EE yet
  • Mike Greiling @mikegreiling commented Apr 01, 2017
    Master

    Here's the MR: gitlab-ce!10262 (merged)

    Here's the MR: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10262
  • Annabel Gray (unavailable until May)
    @annabeldunstone started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Mike Greiling Apr 04, 2017
    app/assets/stylesheets/pages/milestone.scss
    Unable to load the diff.
    • Annabel Gray (unavailable until May) @annabeldunstone commented Apr 03, 2017
      Master

      We need a focus state for keyboard users

      We need a focus state for keyboard users
    Please register or sign in to reply
  • Annabel Gray (unavailable until May)
    @annabeldunstone started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Mike Greiling Apr 03, 2017
    app/assets/stylesheets/pages/milestone.scss
    Unable to load the diff.
    • Annabel Gray (unavailable until May) @annabeldunstone commented Apr 03, 2017
      Master

      This color already exists in variables.scss

      This color already exists in `variables.scss`
    • Annabel Gray (unavailable until May) @annabeldunstone commented Apr 03, 2017
      Master

      @pedroms do we need to add specific color variables that are only going to be used for burndown charts? Could we instead make these less specific and reuse them elsewhere in the app?

      @pedroms do we need to add specific color variables that are only going to be used for burndown charts? Could we instead make these less specific and reuse them elsewhere in the app?
    Please register or sign in to reply
  • Annabel Gray (unavailable until May)
    @annabeldunstone started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Mike Greiling Apr 03, 2017
    app/assets/stylesheets/pages/milestone.scss
    Unable to load the diff.
    • Annabel Gray (unavailable until May) @annabeldunstone commented Apr 03, 2017
      Master

      This color already exists in variables.scss

      This color already exists in `variables.scss`
    Please register or sign in to reply
  • Felipe Artur @felipe_artur commented Apr 03, 2017
    Developer

    @dbalexandre Can you review backend please?

    @dbalexandre Can you review backend please?
  • Mike Greiling @mikegreiling

    added 100 commits

    • 1864d0de...50fd7e13 - 70 commits from branch master
    • 8825a65a - create burndown chart template and js bundle
    • 1ea45f81 - implement burndown chart hint message
    • 767d9a79 - move burndown chart before user action hints
    • 1a230346 - implement burndown chart basic outline
    • dab26c55 - correctly handle empty array passed to setData
    • e2c39ec9 - render burndown chart lines
    • c2257bdf - allow burndown chart to resize during animations
    • 12e24118 - defer render call to the next animation frame to avoid layout thrashing
    • 36027a8a - fix axis font size in burndown chart
    • c44a9d22 - add overflow to the x-axis domain
    • 63fa244e - create x-axis line in constructor instead of regenerating on each render
    • 40e55bd0 - add label to the y-axis
    • 22284ec3 - ideal line should end at dueDate not maxDate in event that milestone goes overdue
    • d1190d62 - create burndown chart legend
    • 4a17b833 - add header to burndown chart and adjust chart size breakpoints
    • 7d76ba45 - add 800ms animation to the line path whenever setData is called
    • 21b19c03 - implement first pass at tooltips feature
    • 9847f60e - improve tooltip design and placement
    • 2e40f0bd - hide tooltips during chart animation
    • 090c9405 - rename resize and animate methods for clarity
    • 47a6524d - remove posessives in comment blocks
    • 5c9778d1 - rename and reorder additional methods for clarity
    • 1292fb03 - update tooltip label through setData options
    • c88b3a8f - fix phantomjs mishandling of svgs with implicit dimensions
    • f446ebc8 - Merge branch 'issue_91' into '91-milestone-burndown-charts'
    • c2d6dd32 - remove test_data and rely on data passed from rails
    • d36d04f4 - add toggle between issue count and issue weight
    • 8595ff79 - Show chart only until current date when milestone due date is bigger
    • 5ae8f237 - adjust burndown chart data toggle style
    • 0d7671e7 - fix end date not matching due date

    Compare with previous version

    Apr 03, 2017

    added 100 commits

    • 1864d0de...50fd7e13 - 70 commits from branch master
    • 8825a65a - create burndown chart template and js bundle
    • 1ea45f81 - implement burndown chart hint message
    • 767d9a79 - move burndown chart before user action hints
    • 1a230346 - implement burndown chart basic outline
    • dab26c55 - correctly handle empty array passed to setData
    • e2c39ec9 - render burndown chart lines
    • c2257bdf - allow burndown chart to resize during animations
    • 12e24118 - defer render call to the next animation frame to avoid layout thrashing
    • 36027a8a - fix axis font size in burndown chart
    • c44a9d22 - add overflow to the x-axis domain
    • 63fa244e - create x-axis line in constructor instead of regenerating on each render
    • 40e55bd0 - add label to the y-axis
    • 22284ec3 - ideal line should end at dueDate not maxDate in event that milestone goes overdue
    • d1190d62 - create burndown chart legend
    • 4a17b833 - add header to burndown chart and adjust chart size breakpoints
    • 7d76ba45 - add 800ms animation to the line path whenever setData is called
    • 21b19c03 - implement first pass at tooltips feature
    • 9847f60e - improve tooltip design and placement
    • 2e40f0bd - hide tooltips during chart animation
    • 090c9405 - rename resize and animate methods for clarity
    • 47a6524d - remove posessives in comment blocks
    • 5c9778d1 - rename and reorder additional methods for clarity
    • 1292fb03 - update tooltip label through setData options
    • c88b3a8f - fix phantomjs mishandling of svgs with implicit dimensions
    • f446ebc8 - Merge branch 'issue_91' into '91-milestone-burndown-charts'
    • c2d6dd32 - remove test_data and rely on data passed from rails
    • d36d04f4 - add toggle between issue count and issue weight
    • 8595ff79 - Show chart only until current date when milestone due date is bigger
    • 5ae8f237 - adjust burndown chart data toggle style
    • 0d7671e7 - fix end date not matching due date

    Compare with previous version

    added 100 commits * 1864d0de...50fd7e13 - 70 commits from branch `master` * 8825a65a - create burndown chart template and js bundle * 1ea45f81 - implement burndown chart hint message * 767d9a79 - move burndown chart before user action hints * 1a230346 - implement burndown chart basic outline * dab26c55 - correctly handle empty array passed to setData * e2c39ec9 - render burndown chart lines * c2257bdf - allow burndown chart to resize during animations * 12e24118 - defer render call to the next animation frame to avoid layout thrashing * 36027a8a - fix axis font size in burndown chart * c44a9d22 - add overflow to the x-axis domain * 63fa244e - create x-axis line in constructor instead of regenerating on each render * 40e55bd0 - add label to the y-axis * 22284ec3 - ideal line should end at dueDate not maxDate in event that milestone goes overdue * d1190d62 - create burndown chart legend * 4a17b833 - add header to burndown chart and adjust chart size breakpoints * 7d76ba45 - add 800ms animation to the line path whenever setData is called * 21b19c03 - implement first pass at tooltips feature * 9847f60e - improve tooltip design and placement * 2e40f0bd - hide tooltips during chart animation * 090c9405 - rename resize and animate methods for clarity * 47a6524d - remove posessives in comment blocks * 5c9778d1 - rename and reorder additional methods for clarity * 1292fb03 - update tooltip label through setData options * c88b3a8f - fix phantomjs mishandling of svgs with implicit dimensions * f446ebc8 - Merge branch 'issue_91' into '91-milestone-burndown-charts' * c2d6dd32 - remove test_data and rely on data passed from rails * d36d04f4 - add toggle between issue count and issue weight * 8595ff79 - Show chart only until current date when milestone due date is bigger * 5ae8f237 - adjust burndown chart data toggle style * 0d7671e7 - fix end date not matching due date [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3705399&start_sha=1864d0de67059f9bfea63311faafad5448b3008d)
    Toggle commit list
  • Mike Greiling @mikegreiling commented Apr 03, 2017
    Master

    @felipe_artur Note: I just rebased on master to pick up some changes from CE that were in the last CE-to-EE merge. Make sure you reset your local branch if you need to commit anything new.

    Hopefully this fixes the spinach test failure. It seems unrelated to our branch.

    @felipe_artur Note: I just rebased on master to pick up some changes from CE that were in the last CE-to-EE merge. Make sure you reset your local branch if you need to commit anything new. Hopefully this fixes the spinach test failure. It seems unrelated to our branch.
  • Mike Greiling @mikegreiling

    added 1 commit

    • b622f464 - adjust color variables and remove focus state css

    Compare with previous version

    Apr 03, 2017

    added 1 commit

    • b622f464 - adjust color variables and remove focus state css

    Compare with previous version

    added 1 commit * b622f464 - adjust color variables and remove focus state css [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3707617&start_sha=0d7671e7b3d1edc3246b55bff3d3845fbade98fb)
    Toggle commit list
  • Mike Greiling
    @mikegreiling started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Mike Greiling Apr 03, 2017
    app/assets/stylesheets/pages/milestone.scss
    Unable to load the diff.
    • Mike Greiling @mikegreiling commented Apr 03, 2017
      Master

      alright, I've removed this line

      alright, I've removed this line
    Please register or sign in to reply
  • Mike Greiling
    @mikegreiling started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Pedro Moreira da Silva Apr 04, 2017
    app/assets/stylesheets/pages/milestone.scss
    Unable to load the diff.
    • Mike Greiling @mikegreiling commented Apr 03, 2017
      Master

      I just replaced all of these with references to already-defined color variables... the only one we don't really have an analog for is the burndown-chart-axis-label-color. I had to settle for a color that wasn't quite as dark, but I don't think it is very noticeable. @pedroms what do you think?

      I just replaced all of these with references to already-defined color variables... the only one we don't really have an analog for is the `burndown-chart-axis-label-color`. I had to settle for a color that wasn't quite as dark, but I don't think it is very noticeable. @pedroms what do you think?
    • Pedro Moreira da Silva @pedroms commented Apr 04, 2017
      Developer

      @mikegreiling yup, sounds good. I trust your color choices 😉

      @mikegreiling yup, sounds good. I trust your color choices :wink:
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Felipe Artur Apr 03, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Can we use the start_date, and end_date over the instance variables?

      Can we use the `start_date`, and `end_date` over the instance variables?
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Can we create a method Burndown#valid? and use it here and here !1540 (diffs)?

      Can we create a method `Burndown#valid?` and use it here and here https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs#f79ee52339500c5eaf333ec18ed015c9482b8806_0_19?
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Felipe Artur Apr 03, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      This seems a bit weird to me since you are selecting only closed issues here !1540 (diffs). Should not be closed_count?

      Derp, I notice that you are assigning this value in the initializer. Can we rename then to open_issues_count and open_issues_weight and use the accessor methods?

      Edited Apr 03, 2017 by Douglas Barbosa Alexandre
      ~~This seems a bit weird to me since you are selecting only closed issues here https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs#f79ee52339500c5eaf333ec18ed015c9482b8806_0_56. Should not be `closed_count`?~~ Derp, I notice that you are assigning this value in the initializer. Can we rename then to `open_issues_count` and `open_issues_weight` and use the accessor methods?
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Felipe Artur Apr 03, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Can we rename this to closed_issues?

      Can we rename this to `closed_issues`?
    • Felipe Artur @felipe_artur commented Apr 03, 2017
      Developer

      these are not closed issues, they can be opened.
      We should rename to issues_with_closed_at

      these are not closed issues, they can be opened. We should rename to `issues_with_closed_at`
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Felipe Artur Apr 03, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Should not be @milestone.issues.only_opened?

      Should not be `@milestone.issues.only_opened`?
    • Felipe Artur @felipe_artur commented Apr 03, 2017
      Developer

      No. We also need the closed ones here.
      We calculate the All issues - closed issues per day

      No. We also need the closed ones here. We calculate the `All issues - closed issues` per day
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Can we rename this to make its intent more clear?

      Can we rename this to make its intent more clear?
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Felipe Artur Apr 03, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Can we rename the method to make sense with the order of the return values?

      Can we rename the method to make sense with the order of the return values?
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Felipe Artur Apr 03, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      I think that count_and_weight_of is doing too much here. Wdyt?

      closed_issues_count = closed_issues.count
      closed_issues_weight = sum_issues_weight(closed_issues)
      I think that `count_and_weight_of ` is doing too much here. Wdyt? ```ruby closed_issues_count = closed_issues.count closed_issues_weight = sum_issues_weight(closed_issues) ```
    • Felipe Artur @felipe_artur commented Apr 03, 2017
      Developer

      I don't have a strong opinion against it. We can change anyway...

      I don't have a strong opinion against it. We can change anyway...
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Felipe Artur Apr 03, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Same here:

      reopened_issues_count = reopened_issues.count
      reopened_issues_weight = sum_issues_weight(reopened_issues)
      Same here: ```ruby reopened_issues_count = reopened_issues.count reopened_issues_weight = sum_issues_weight(reopened_issues) ```
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Felipe Artur Apr 03, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      See !1540 (diffs, comment 26750997)

      See https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs#note_26750997
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Felipe Artur Apr 03, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Can we use issues.sum(:weight)?

      Can we use `issues.sum(:weight)`?
    • Felipe Artur @felipe_artur commented Apr 03, 2017
      Developer

      No. This is not a relation.

      No. This is not a relation.
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Mike Greiling Apr 05, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Can we rename this to reopened_and_closed_issues_by_data(date)?

      Can we rename this to `reopened_and_closed_issues_by_data(date)`?
    • Felipe Artur @felipe_artur commented Apr 03, 2017
      Developer

      renamed to closed_and_reopened_issues_by(date) i think it is less idiomatic to repeat date eg:by_date(date) in the parameter.

      renamed to `closed_and_reopened_issues_by(date)` i think it is less idiomatic to repeat date eg:`by_date(date)` in the parameter.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      I think the closed_and_reopened_issues_by raises the question "by what".

      I think the `closed_and_reopened_issues_by` raises the question "by what".
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on the diff Apr 03, 2017
    Resolved by Felipe Artur Apr 04, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Can we use closed.only_reopened?

      Can we use `closed.only_reopened`?
    • Felipe Artur @felipe_artur commented Apr 03, 2017
      Developer

      No because it is not a relation.

      I think to do like this we would need to use #where instead of #select and that will require to trunc the date, which are two different methods for Postgres and Mysql.

      Edited Apr 03, 2017 by Felipe Artur
      No because it is not a relation. I think to do like this we would need to use `#where` instead of `#select` and that will require to trunc the date, which are two different methods for Postgres and Mysql.
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on the diff Apr 03, 2017
    Resolved by Mike Greiling Apr 05, 2017
    app/models/issue.rb
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Why did you remove this?

      Edited Apr 03, 2017 by Douglas Barbosa Alexandre
      Why did you remove this?
    • Felipe Artur @felipe_artur commented Apr 03, 2017
      Developer

      To follow a condition of burndown:

      If d > i.closed_at && i.state(now)==open, then assume i was opened on i.closed_at + 1 day. So do count/weight it.

      if we set closed_at to nil when reopening an issue we won't be able to count as reopened on burndown.

      To follow a condition of burndown: > If `d > i.closed_at && i.state(now)==open`, then _assume_ `i` was opened on `i.closed_at + 1 day`. So _**do**_ count/weight it. if we set closed_at to nil when reopening an issue we won't be able to count as reopened on burndown.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Thanks for explaining why 👍

      Thanks for explaining why :+1:
    • Sean McGivern @smcgivern commented Apr 04, 2017
      Master

      Do we need to remove this in CE too?

      Do we need to remove this in CE too?
    • Mike Greiling @mikegreiling commented Apr 04, 2017
      Master

      Yes, this should be backported to CE

      Yes, this should be backported to CE
    • Mike Greiling @mikegreiling commented Apr 04, 2017
      Master

      likewise with the changes in spec/models/issue_spec.rb

      likewise with the changes in `spec/models/issue_spec.rb`
    • Felipe Artur @felipe_artur commented Apr 04, 2017
      Developer

      Yes, if a CE user upgrades to EE the data for burndown will be more accurate.

      Yes, if a CE user upgrades to EE the data for burndown will be more accurate.
    • Felipe Artur @felipe_artur commented Apr 04, 2017
      Developer

      @smcgivern The backport MR gitlab-ce!10453 (merged)

      @smcgivern The backport MR https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10453
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Felipe Artur Apr 03, 2017
    app/views/shared/milestones/_burndown.html.haml 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      See !1540 (diffs, comment 26751346)

      See https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs#note_26751346
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Felipe Artur Apr 03, 2017
    spec/models/burndown_spec.rb 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Can we use create_list(:issues, count, ...)?

      Can we use `create_list(:issues, count, ...)`?
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Felipe Artur Apr 03, 2017
    spec/models/burndown_spec.rb 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Can we use closed.each(&:closed)?

      Can we use `closed.each(&:closed)`?
    Please register or sign in to reply
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Felipe Artur Apr 03, 2017
    spec/models/burndown_spec.rb 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Can we use closed.slice(0..count / 4).each(&: reopen)?

      Can we use `closed.slice(0..count / 4).each(&: reopen)`?
    • Mike Greiling @mikegreiling commented Apr 03, 2017
      Master

      is there supposed to be a space between &: and the method name?

      is there supposed to be a space between `&:` and the method name?
    • Felipe Artur @felipe_artur commented Apr 03, 2017
      Developer

      nope, probably a typo...

      Edited Apr 03, 2017 by Felipe Artur
      nope, probably a typo...
    Please register or sign in to reply
  • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
    Master

    Thanks @felipe_artur! I left some comments.

    Thanks @felipe_artur! I left some comments.
  • Felipe Artur @felipe_artur commented Apr 03, 2017
    Developer

    Thanks for your help @dbalexandre!!!

    @smcgivern Can you review please?

    Thanks for your help @dbalexandre!!! @smcgivern Can you review please?
  • Felipe Artur @felipe_artur

    added 1 commit

    • 31ca765f - Small changes to burndown model and specs

    Compare with previous version

    Apr 03, 2017

    added 1 commit

    • 31ca765f - Small changes to burndown model and specs

    Compare with previous version

    added 1 commit * 31ca765f - Small changes to burndown model and specs [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3717252&start_sha=b622f464f97f3647665d69632b927edfe9fa8e93)
    Toggle commit list
  • Douglas Barbosa Alexandre
    @dbalexandre started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Felipe Artur Apr 03, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Douglas Barbosa Alexandre @dbalexandre commented Apr 03, 2017
      Master

      Wdyt issues.map(&:weight).compact.reduce(:+)?

      Wdyt `issues.map(&:weight).compact.reduce(:+)`?
    Please register or sign in to reply
  • Felipe Artur @felipe_artur

    added 1 commit

    • f87c9f6a - Small changes to burndown model and specs

    Compare with previous version

    Apr 03, 2017

    added 1 commit

    • f87c9f6a - Small changes to burndown model and specs

    Compare with previous version

    added 1 commit * f87c9f6a - Small changes to burndown model and specs [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3719044&start_sha=31ca765fbabbe9fa524f07cef56a8665cafd71f6)
    Toggle commit list
  • Felipe Artur @felipe_artur

    added 1 commit

    • 2a048b17 - Small changes to burndown model and specs

    Compare with previous version

    Apr 03, 2017

    added 1 commit

    • 2a048b17 - Small changes to burndown model and specs

    Compare with previous version

    added 1 commit * 2a048b17 - Small changes to burndown model and specs [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3719396&start_sha=f87c9f6a734470fb59269a878e22358af0534b1d)
    Toggle commit list
  • Felipe Artur @felipe_artur

    added 1 commit

    • 1bd74265 - Small changes to burndown model and specs

    Compare with previous version

    Apr 03, 2017

    added 1 commit

    • 1bd74265 - Small changes to burndown model and specs

    Compare with previous version

    added 1 commit * 1bd74265 - Small changes to burndown model and specs [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3719433&start_sha=2a048b17b1e8826708acac52f328da9e65c46adf)
    Toggle commit list
  • Felipe Artur
    @felipe_artur started a discussion on an old version of the diff Apr 03, 2017
    Resolved by Mike Greiling Apr 03, 2017
    app/views/shared/milestones/_burndown.html.haml 0 → 100644
    Unable to load the diff.
    • Felipe Artur @felipe_artur commented Apr 03, 2017
      Developer

      we could change this one and eliminate can_generate_chart variable

      - if @burndown.valid?  
      # Generate chart
      - elsif can?(current_user, :admin_milestone, @project) && cookies['hide_burndown_message'].nil?
      # show zero state panel

      @mikegreiling

      Edited Apr 03, 2017 by Felipe Artur
      we could change this one and eliminate `can_generate_chart` variable ``` ruby - if @burndown.valid? # Generate chart - elsif can?(current_user, :admin_milestone, @project) && cookies['hide_burndown_message'].nil? # show zero state panel ``` @mikegreiling
    • Mike Greiling @mikegreiling commented Apr 03, 2017
      Master

      👍

      :+1:
    Please register or sign in to reply
  • Mike Greiling @mikegreiling

    added 1 commit

    • c48f972b - fix simulate_drag issues

    Compare with previous version

    Apr 03, 2017

    added 1 commit

    • c48f972b - fix simulate_drag issues

    Compare with previous version

    added 1 commit * c48f972b - fix simulate_drag issues [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3720240&start_sha=1bd7426503c8a69c93e4add4b7a8823409e0a6a2)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 1 commit

    • cfeadfb2 - clean up burndown empty state logic

    Compare with previous version

    Apr 03, 2017

    added 1 commit

    • cfeadfb2 - clean up burndown empty state logic

    Compare with previous version

    added 1 commit * cfeadfb2 - clean up burndown empty state logic [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3720321&start_sha=c48f972b2e65a1cac49708ea6b4d2eec2566d8f4)
    Toggle commit list
  • Mike Greiling @mikegreiling

    mentioned in merge request gitlab-ce!10437 (merged)

    Apr 04, 2017

    mentioned in merge request gitlab-ce!10437 (merged)

    mentioned in merge request gitlab-ce!10437
    Toggle commit list
  • Sean McGivern
    @smcgivern started a discussion on an old version of the diff Apr 04, 2017
    Resolved by Felipe Artur Apr 04, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Sean McGivern @smcgivern commented Apr 04, 2017
      Master

      reduce(:+) -> sum!

      `reduce(:+)` -> `sum`!
    • Sean McGivern @smcgivern commented Apr 04, 2017
      Master

      We also don't need the || 0 if we use sum 🙂

      We also don't need the `|| 0` if we use `sum` :slight_smile:
    Please register or sign in to reply
  • Sean McGivern
    @smcgivern started a discussion on an old version of the diff Apr 04, 2017
    Resolved by Felipe Artur Apr 05, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Sean McGivern @smcgivern commented Apr 04, 2017
      Master

      Don't need return!

      Don't need `return`!
    • Mike Greiling @mikegreiling commented Apr 04, 2017
      Master

      is an explicit return needed when you are returning a tuple like this, maybe?

      is an explicit `return` needed when you are returning a tuple like this, maybe?
    • Felipe Artur @felipe_artur commented Apr 04, 2017
      Developer

      Yes, it is needed when returning multiple values.

      Yes, it is needed when returning multiple values.
    • Sean McGivern @smcgivern commented Apr 05, 2017
      Master

      Or it could just be [closed, reopened] 🙂 Sorry, that was very terse of me - would you mind changing it to that?

      Or it could just be `[closed, reopened]` :slight_smile: Sorry, that was very terse of me - would you mind changing it to that?
    Please register or sign in to reply
  • Sean McGivern
    @smcgivern started a discussion on an old version of the diff Apr 04, 2017
    Resolved by Felipe Artur Apr 04, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Sean McGivern @smcgivern commented Apr 04, 2017
      Master

      Does this run two queries? We could do milestone.issues.reorder(nil).pluck('COUNT(*), SUM(weight)'), but I'm not sure if it's worth it.

      Does this run two queries? We could do ` milestone.issues.reorder(nil).pluck('COUNT(*), SUM(weight)')`, but I'm not sure if it's worth it.
    • Felipe Artur @felipe_artur commented Apr 04, 2017
      Developer

      milestone.issues.reorder(nil).pluck('COUNT(*), COALESCE(SUM(weight), 0)') does the job.

      I think it is better to have just one query.

      `milestone.issues.reorder(nil).pluck('COUNT(*), COALESCE(SUM(weight), 0)')` does the job. I think it is better to have just one query.
    Please register or sign in to reply
  • Sean McGivern
    @smcgivern started a discussion on an old version of the diff Apr 04, 2017
    Resolved by Felipe Artur Apr 04, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Sean McGivern @smcgivern commented Apr 04, 2017
      Master

      Please indent this two spaces more, as it's a continuation!

      Please indent this two spaces more, as it's a continuation!
    • Felipe Artur @felipe_artur commented Apr 04, 2017
      Developer

      it is idented two spaces more, do you mean four spaces?

      it is idented two spaces more, do you mean four spaces?
    Please register or sign in to reply
  • Sean McGivern
    @smcgivern started a discussion on an old version of the diff Apr 04, 2017
    Resolved by Felipe Artur Apr 04, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Sean McGivern @smcgivern commented Apr 04, 2017
      Master

      I think we should memoize to an instance variable that matches the method name.

      I think we should memoize to an instance variable that matches the method name.
    Please register or sign in to reply
  • Sean McGivern
    @smcgivern started a discussion on an old version of the diff Apr 04, 2017
    Resolved by Felipe Artur Apr 04, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Sean McGivern @smcgivern commented Apr 04, 2017
      Master

      Why do we need the to_i on both sides? Could we just use to_date instead of .beginning_of_day.to_i?

      Why do we need the `to_i` on both sides? Could we just use `to_date` instead of `.beginning_of_day.to_i`?
    Please register or sign in to reply
  • Sean McGivern
    @smcgivern started a discussion on an old version of the diff Apr 04, 2017
    Resolved by Felipe Artur Apr 04, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Sean McGivern @smcgivern commented Apr 04, 2017
      Master

      Can we pull the assignments out of the array? It's quite confusing otherwise 🙂

      Can we pull the assignments out of the array? It's quite confusing otherwise :slight_smile:
    Please register or sign in to reply
  • Sean McGivern
    @smcgivern started a discussion on an old version of the diff Apr 04, 2017
    Resolved by Sean McGivern Apr 05, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Sean McGivern @smcgivern commented Apr 04, 2017
      Master

      Do we actually need the ordering?

      Do we actually need the ordering?
    • Felipe Artur @felipe_artur commented Apr 04, 2017
      Developer

      Since #select iterates over the array if the dates come first it will probably be faster, at least for small arrays.
      But i don't know if using order will take that advantage off.

                     static VALUE
      rb_ary_select_bang(VALUE ary)
      {
          long i1, i2;
      
          RETURN_SIZED_ENUMERATOR(ary, 0, 0, ary_enum_length);
          rb_ary_modify(ary);
          for (i1 = i2 = 0; i1 < RARRAY_LEN(ary); i1++) {
              VALUE v = RARRAY_AREF(ary, i1);
              if (!RTEST(rb_yield(v))) continue;
              if (i1 != i2) {
                  rb_ary_store(ary, i2, v);
              }
              i2++;
          }
      
          if (i1 == i2) return Qnil;
          if (i2 < i1)
              ARY_SET_LEN(ary, i2);
          return ary;
      }
      
      Since `#select` iterates over the array if the dates come first it will probably be faster, at least for small arrays. But i don't know if using order will take that advantage off. ``` c static VALUE rb_ary_select_bang(VALUE ary) { long i1, i2; RETURN_SIZED_ENUMERATOR(ary, 0, 0, ary_enum_length); rb_ary_modify(ary); for (i1 = i2 = 0; i1 < RARRAY_LEN(ary); i1++) { VALUE v = RARRAY_AREF(ary, i1); if (!RTEST(rb_yield(v))) continue; if (i1 != i2) { rb_ary_store(ary, i2, v); } i2++; } if (i1 == i2) return Qnil; if (i2 < i1) ARY_SET_LEN(ary, i2); return ary; } ```
    • Sean McGivern @smcgivern commented Apr 05, 2017
      Master

      OK, it's not super-important anyway 🙂

      OK, it's not super-important anyway :slight_smile:
    Please register or sign in to reply
  • Sean McGivern
    @smcgivern started a discussion on an old version of the diff Apr 04, 2017
    Resolved by Mike Greiling Apr 04, 2017
    app/views/shared/milestones/_burndown.html.haml 0 → 100644
    Unable to load the diff.
    • Sean McGivern @smcgivern commented Apr 04, 2017
      Master

      can_generate_chart!

      `can_generate_chart`!
    • Mike Greiling @mikegreiling commented Apr 04, 2017
      Master

      oops, forgot to remove that line

      oops, forgot to remove that line
    Please register or sign in to reply
  • Sean McGivern
    @smcgivern started a discussion on an old version of the diff Apr 04, 2017
    Resolved by Felipe Artur Apr 04, 2017
    app/views/shared/milestones/_burndown.html.haml 0 → 100644
    Unable to load the diff.
    • Sean McGivern @smcgivern commented Apr 04, 2017
      Master

      Could these start and end dates not be part of burndown.chart_data?

      Could these start and end dates not be part of `burndown.chart_data`?
    • Mike Greiling @mikegreiling commented Apr 04, 2017
      Master

      The chart_data is a simple array of values... to make it a part of that would require turning it into a hash and then adopting a schema of some sort. I think it is simpler to just do it this way.

      The chart_data is a simple array of values... to make it a part of that would require turning it into a hash and then adopting a schema of some sort. I think it is simpler to just do it this way.
    • Sean McGivern @smcgivern commented Apr 04, 2017
      Master

      An array with items in a specific order also has a schema, though! Really, we should use whatever data structure makes most sense. Here it seems weird that we're passing this data from the milestone rather than the burndown, because the burndown should have all the information we need.

      An array with items in a specific order also has a schema, though! Really, we should use whatever data structure makes most sense. Here it seems weird that we're passing this data from the `milestone` rather than the `burndown`, because the `burndown` should have all the information we need.
    • Mike Greiling @mikegreiling commented Apr 04, 2017
      Master

      the due_date cannot be introspected from the array of points that chart_data gives us because it may be a milestone in-progress. we'd still need a way to specify the start and end dates separately...

      we could change chart_data to output a hash like:

      {
        start_date: "2017-03-01",
        due_date: "2017-04-01",
        data: [ ... ]
      }

      but that's not really much different from what we're doing here and I'd argue it would be a bit more arcane...

      as for using burndown.due_date instead of milestone.due_date yeah I think that would be a good idea.

      Edited Apr 04, 2017 by Mike Greiling
      the due_date cannot be introspected from the array of points that chart_data gives us because it may be a milestone in-progress. we'd still need a way to specify the start and end dates separately... we could change chart_data to output a hash like: ```ruby { start_date: "2017-03-01", due_date: "2017-04-01", data: [ ... ] } ``` but that's not really much different from what we're doing here and I'd argue it would be a bit more arcane... as for using `burndown.due_date` instead of `milestone.due_date` yeah I think that would be a good idea.
    • Sean McGivern @smcgivern commented Apr 04, 2017
      Master

      @mikegreiling right, that's what I mean. I don't see why the object is worse, and then the backend can return a single object which contains all the information we need, instead of having most of the information from one method, and the rest from another.

      @mikegreiling right, that's what I mean. I don't see why the object is worse, and then the backend can return a single object which contains all the information we need, instead of having most of the information from one method, and the rest from another.
    • Felipe Artur @felipe_artur commented Apr 04, 2017
      Developer

      Burndown has these dates accessible already.

      `Burndown` has these dates accessible already.
    Please register or sign in to reply
  • Sean McGivern
    @smcgivern started a discussion on an old version of the diff Apr 04, 2017
    Resolved by Felipe Artur Apr 04, 2017
    app/models/burndown.rb 0 → 100644
    Unable to load the diff.
    • Sean McGivern @smcgivern commented Apr 04, 2017
      Master

      If we made this the as_json method, we could just do burndown.to_json in the view. Up to you 🙂

      If we made this the `as_json` method, we could just do `burndown.to_json` in the view. Up to you :slight_smile:
    • Felipe Artur @felipe_artur commented Apr 04, 2017
      Developer

      It was on my initial plans, forgot about it. Thx!

      It was on my initial plans, forgot about it. Thx!
    Please register or sign in to reply
  • Sean McGivern @smcgivern commented Apr 04, 2017
    Master

    @felipe_artur @mikegreiling thanks! Just out of interest, I ran this on staging for 8.17, using the updated_at column instead of closed_at. Here's what I got 🙂

    irb(main):103:0> Burndown.new(milestone).chart_data
    D, [2017-04-04T10:27:21.847368 #42571] DEBUG -- :    (1.4ms)  SELECT COUNT(*) FROM "issues" WHERE "issues"."deleted_at" IS NULL AND "issues"."milestone_id" = $1  [["milestone_id", 134231]]
    D, [2017-04-04T10:27:21.848826 #42571] DEBUG -- :    (0.9ms)  SELECT SUM("issues"."weight") FROM "issues" WHERE "issues"."deleted_at" IS NULL AND "issues"."milestone_id" = $1  [["milestone_id", 134231]]
    D, [2017-04-04T10:27:21.851861 #42571] DEBUG -- :   Issue Load (1.6ms)  SELECT updated_at AS closed_at, weight, state FROM "issues" WHERE "issues"."deleted_at" IS NULL AND "issues"."milestone_id" = $1 AND (updated_at IS NOT NULL)  ORDER BY "issues"."id" DESC, updated_at ASC  [["milestone_id", 134231]]
    => [["2017-01-22", 165, 7], ["2017-01-23", 160, 7], ["2017-01-24", 152, 7], ["2017-01-25", 146, 7], ["2017-01-26", 139, 7], ["2017-01-27", 133, 7], ["2017-01-28", 133, 7], ["2017-01-29", 130, 7], ["2017-01-30", 125, 7], ["2017-01-31", 121, 7], ["2017-02-01", 117, 7], ["2017-02-02", 108, 7], ["2017-02-03", 98, 7], ["2017-02-04", 97, 7], ["2017-02-05", 97, 7], ["2017-02-06", 90, 7], ["2017-02-07", 73, 3], ["2017-02-08", 40, 2], ["2017-02-09", 28, 2], ["2017-02-10", 17, 1], ["2017-02-11", 15, 1], ["2017-02-12", 15, 1], ["2017-02-13", 15, 1], ["2017-02-14", 15, 1], ["2017-02-15", 15, 1], ["2017-02-16", 15, 1], ["2017-02-17", 15, 1], ["2017-02-18", 15, 1], ["2017-02-19", 15, 1], ["2017-02-20", 15, 1], ["2017-02-21", 15, 1], ["2017-02-22", 15, 1]]
    @felipe_artur @mikegreiling thanks! Just out of interest, I ran this on staging for 8.17, using the `updated_at` column instead of `closed_at`. Here's what I got :slight_smile: ```ruby irb(main):103:0> Burndown.new(milestone).chart_data D, [2017-04-04T10:27:21.847368 #42571] DEBUG -- : (1.4ms) SELECT COUNT(*) FROM "issues" WHERE "issues"."deleted_at" IS NULL AND "issues"."milestone_id" = $1 [["milestone_id", 134231]] D, [2017-04-04T10:27:21.848826 #42571] DEBUG -- : (0.9ms) SELECT SUM("issues"."weight") FROM "issues" WHERE "issues"."deleted_at" IS NULL AND "issues"."milestone_id" = $1 [["milestone_id", 134231]] D, [2017-04-04T10:27:21.851861 #42571] DEBUG -- : Issue Load (1.6ms) SELECT updated_at AS closed_at, weight, state FROM "issues" WHERE "issues"."deleted_at" IS NULL AND "issues"."milestone_id" = $1 AND (updated_at IS NOT NULL) ORDER BY "issues"."id" DESC, updated_at ASC [["milestone_id", 134231]] => [["2017-01-22", 165, 7], ["2017-01-23", 160, 7], ["2017-01-24", 152, 7], ["2017-01-25", 146, 7], ["2017-01-26", 139, 7], ["2017-01-27", 133, 7], ["2017-01-28", 133, 7], ["2017-01-29", 130, 7], ["2017-01-30", 125, 7], ["2017-01-31", 121, 7], ["2017-02-01", 117, 7], ["2017-02-02", 108, 7], ["2017-02-03", 98, 7], ["2017-02-04", 97, 7], ["2017-02-05", 97, 7], ["2017-02-06", 90, 7], ["2017-02-07", 73, 3], ["2017-02-08", 40, 2], ["2017-02-09", 28, 2], ["2017-02-10", 17, 1], ["2017-02-11", 15, 1], ["2017-02-12", 15, 1], ["2017-02-13", 15, 1], ["2017-02-14", 15, 1], ["2017-02-15", 15, 1], ["2017-02-16", 15, 1], ["2017-02-17", 15, 1], ["2017-02-18", 15, 1], ["2017-02-19", 15, 1], ["2017-02-20", 15, 1], ["2017-02-21", 15, 1], ["2017-02-22", 15, 1]] ```
  • Mike Greiling @mikegreiling

    added 3 commits

    • bc7d2588 - clean up burndown empty state logic
    • 3cd662fc - Port 'refactor_simulate_drag' to EE
    • 0f946245 - convert capitalization scheme to sentence case

    Compare with previous version

    Apr 04, 2017

    added 3 commits

    • bc7d2588 - clean up burndown empty state logic
    • 3cd662fc - Port 'refactor_simulate_drag' to EE
    • 0f946245 - convert capitalization scheme to sentence case

    Compare with previous version

    added 3 commits * bc7d2588 - clean up burndown empty state logic * 3cd662fc - Port &#x27;refactor_simulate_drag&#x27; to EE * 0f946245 - convert capitalization scheme to sentence case [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3734723&start_sha=cfeadfb28b8aa8be4aa68daee1957e38de6846f7)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 1 commit

    • 5757480c - fix unused variable

    Compare with previous version

    Apr 04, 2017

    added 1 commit

    • 5757480c - fix unused variable

    Compare with previous version

    added 1 commit * 5757480c - fix unused variable [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3734922&start_sha=0f9462459828a07855327a4800026b4f3fdc690a)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 4 commits

    • 6056cddb - Port 'refactor_simulate_drag' to EE
    • d0d9980c - convert capitalization scheme to sentence case
    • d9c7034c - fix unused variable
    • 47ce88a3 - Merge branch 'refactor_simulate_drag-ee' into 91-milestone-burndown-charts

    Compare with previous version

    Apr 04, 2017

    added 4 commits

    • 6056cddb - Port 'refactor_simulate_drag' to EE
    • d0d9980c - convert capitalization scheme to sentence case
    • d9c7034c - fix unused variable
    • 47ce88a3 - Merge branch 'refactor_simulate_drag-ee' into 91-milestone-burndown-charts

    Compare with previous version

    added 4 commits * 6056cddb - Port &#x27;refactor_simulate_drag&#x27; to EE * d0d9980c - convert capitalization scheme to sentence case * d9c7034c - fix unused variable * 47ce88a3 - Merge branch &#x27;refactor_simulate_drag-ee&#x27; into 91-milestone-burndown-charts [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3736387&start_sha=5757480ca25771d1a675ef0bc541ea53debb3a54)
    Toggle commit list
  • Felipe Artur @felipe_artur

    added 1 commit

    • aa04757a - Code improvements on burndown model and view partial

    Compare with previous version

    Apr 04, 2017

    added 1 commit

    • aa04757a - Code improvements on burndown model and view partial

    Compare with previous version

    added 1 commit * aa04757a - Code improvements on burndown model and view partial [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3737786&start_sha=47ce88a33e22a23272f568732882554f1c77e425)
    Toggle commit list
  • Felipe Artur @felipe_artur

    mentioned in merge request gitlab-ce!10453 (merged)

    Apr 04, 2017

    mentioned in merge request gitlab-ce!10453 (merged)

    mentioned in merge request gitlab-ce!10453
    Toggle commit list
  • Felipe Artur @felipe_artur commented Apr 04, 2017
    Developer

    @smcgivern Done. Can you take a look again please?

    @smcgivern Done. Can you take a look again please?
  • Achilleas Pipinellis (🌴 back on April 30 🌴) @axil commented Apr 04, 2017
    Master

    I removed the Closes #91 reference since we still need docs for this :)

    _I removed the `Closes #91` reference since we still need docs for this :)_
  • Mike Greiling @mikegreiling

    added 2 commits

    • db6fb490 - use a less memory-intensive sourcemap when running in CI
    • 0cf67a93 - Merge branch 'refactor_simulate_drag-ee' into 91-milestone-burndown-charts

    Compare with previous version

    Apr 04, 2017

    added 2 commits

    • db6fb490 - use a less memory-intensive sourcemap when running in CI
    • 0cf67a93 - Merge branch 'refactor_simulate_drag-ee' into 91-milestone-burndown-charts

    Compare with previous version

    added 2 commits * db6fb490 - use a less memory-intensive sourcemap when running in CI * 0cf67a93 - Merge branch &#x27;refactor_simulate_drag-ee&#x27; into 91-milestone-burndown-charts [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3739152&start_sha=aa04757a2681c9b17d00f845593f292a92bfdc2e)
    Toggle commit list
  • Jacob Schatz @jschatz1 commented Apr 04, 2017
    Master

    As I mentioned in our call and as you discovered, I believe the dotted line needs to go to the end of the milestone not to the end of today.

    Screen_Shot_2017-04-04_at_5.05.56_PM

    As I mentioned in our call and as you discovered, I believe the dotted line needs to go to the end of the milestone not to the end of today. ![Screen_Shot_2017-04-04_at_5.05.56_PM](/uploads/afd47778fa4df6a57168afec8640750e/Screen_Shot_2017-04-04_at_5.05.56_PM.png)
  • Mike Greiling @mikegreiling

    added 1 commit

    • ae8ef069 - don't confuse the due date and the last data entry date

    Compare with previous version

    Apr 04, 2017

    added 1 commit

    • ae8ef069 - don't confuse the due date and the last data entry date

    Compare with previous version

    added 1 commit * ae8ef069 - don&#x27;t confuse the due date and the last data entry date [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3739877&start_sha=0cf67a9368b8823b001cf94796146cd70de62722)
    Toggle commit list
  • Mike Greiling @mikegreiling

    added 1 commit

    • 38a666e6 - fix "Undefined method `+' for nil:NilClass" error

    Compare with previous version

    Apr 04, 2017

    added 1 commit

    • 38a666e6 - fix "Undefined method `+' for nil:NilClass" error

    Compare with previous version

    added 1 commit * 38a666e6 - fix &quot;Undefined method `+&#x27; for nil:NilClass&quot; error [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3740013&start_sha=ae8ef069be425bfa0de09257cbcdd70c65528ea8)
    Toggle commit list
  • Jacob Schatz @jschatz1 commented Apr 04, 2017
    Master

    Looks fixed @mikegreiling

    ss__2017-04-04_at_5.46.19_PM

    Looks fixed @mikegreiling ![ss__2017-04-04_at_5.46.19_PM](/uploads/eb0066bf83c5509368af498a9cf62174/ss__2017-04-04_at_5.46.19_PM.png)
  • Mike Greiling @mikegreiling

    added 158 commits

    • 38a666e6...2019c376 - 157 commits from branch master
    • 7f577bda - Merge branch 'master' into 91-milestone-burndown-charts

    Compare with previous version

    Apr 04, 2017

    added 158 commits

    • 38a666e6...2019c376 - 157 commits from branch master
    • 7f577bda - Merge branch 'master' into 91-milestone-burndown-charts

    Compare with previous version

    added 158 commits * 38a666e6...2019c376 - 157 commits from branch `master` * 7f577bda - Merge branch &#x27;master&#x27; into 91-milestone-burndown-charts [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3740327&start_sha=38a666e62fdf52c0eddac7d66251eed1a4049aa0)
    Toggle commit list
  • Jacob Schatz
    @jschatz1 started a discussion on an old version of the diff Apr 05, 2017
    Resolved by Mike Greiling Apr 05, 2017
    app/assets/javascripts/burndown_chart/index.js 0 → 100644
    Unable to load the diff.
    • Jacob Schatz @jschatz1 commented Apr 05, 2017
      Master

      Generally like to name jquery things to $nameOfConst

      Generally like to name jquery things to `$nameOfConst`
    • Mike Greiling @mikegreiling commented Apr 05, 2017
      Master

      done

      done
    Please register or sign in to reply
  • Jacob Schatz
    @jschatz1 started a discussion on the diff Apr 05, 2017
    Resolved by Mike Greiling Apr 05, 2017
    app/assets/javascripts/burndown_chart/index.js 0 → 100644
    Unable to load the diff.
    • Jacob Schatz @jschatz1 commented Apr 05, 2017
      Master

      I am fine with this.

      I am fine with this.
    • Mike Greiling @mikegreiling commented Apr 05, 2017
      Master

      👍

      :+1:
    • Mike Greiling @mikegreiling commented Apr 05, 2017
      Master

      I was pretty careful to read-only on these resize callbacks and defer DOM manipulation to requestAnimationFrame callbacks so as to not cause any layout thrashing.

      I was pretty careful to read-only on these resize callbacks and defer DOM manipulation to `requestAnimationFrame` callbacks so as to not cause any layout thrashing.
    • Jacob Schatz @jschatz1 commented Apr 05, 2017
      Master

      Should we debounce?

      Edited Apr 05, 2017
      Should we `debounce`?
    • Mike Greiling @mikegreiling commented Apr 05, 2017
      Master

      I think it should be fine as it is. animateResize is a very lightweight function that works similarly to a debounce, but is a bit better suited for a resize callback. It won't wait for the debounce function to stop being called, but it does prevent the resize callback from causing any more than 1 re-render per 50ms.

      So if a resize event calls animateResize 10 times in 50ms, it will only trigger resize one time, and resize will only schedule a render if the size has changed. It should be pretty performant.

      I can alter the 50ms threshold to something higher if it becomes necessary.

      Edited Apr 05, 2017
      I think it should be fine as it is. `animateResize` is a very lightweight function that works similarly to a debounce, but is a bit better suited for a `resize` callback. It won't wait for the debounce function to stop being called, but it does prevent the resize callback from causing any more than 1 re-render per 50ms. So if a `resize` event calls `animateResize` 10 times in 50ms, it will only trigger `resize` one time, and `resize` will only schedule a `render` if the size has changed. It should be pretty performant. I can alter the 50ms threshold to something higher if it becomes necessary.
    Please register or sign in to reply
  • Jacob Schatz
    @jschatz1 started a discussion on an old version of the diff Apr 05, 2017
    Resolved by Mike Greiling Apr 05, 2017
    app/assets/javascripts/burndown_chart/index.js 0 → 100644
    Unable to load the diff.
    • Jacob Schatz @jschatz1 commented Apr 05, 2017
      Master

      is this specific enough?

      Edited Apr 05, 2017 by Jacob Schatz
      is this specific enough?
    • Mike Greiling @mikegreiling commented Apr 05, 2017
      Master

      I'm adding a .js-burndown-data-selector class to make this more clear.

      I'm adding a `.js-burndown-data-selector` class to make this more clear.
    Please register or sign in to reply
  • Jacob Schatz @jschatz1 commented Apr 05, 2017
    Master

    @mikegreiling I am good with this minus 1 or 2 comments

    @mikegreiling I am good with this minus 1 or 2 comments
  • Mike Greiling @mikegreiling

    added 2 commits

    • 796158c3 - follow jQuery variable $someConst convention
    • 163a87a7 - make a custom class for the burndown data toggle

    Compare with previous version

    Apr 05, 2017

    added 2 commits

    • 796158c3 - follow jQuery variable $someConst convention
    • 163a87a7 - make a custom class for the burndown data toggle

    Compare with previous version

    added 2 commits * 796158c3 - follow jQuery variable $someConst convention * 163a87a7 - make a custom class for the burndown data toggle [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3741747&start_sha=7f577bda318294ccad6194c49265b66760918aaa)
    Toggle commit list
  • Sean McGivern @smcgivern commented Apr 05, 2017
    Master

    @axil thank you, that's my mistake. Who is writing the docs for this @felipe_artur @mikegreiling? Can you create an empty follow-up MR so we don't lose track of this?

    @axil thank you, that's my mistake. Who is writing the docs for this @felipe_artur @mikegreiling? Can you create an empty follow-up MR so we don't lose track of this?
  • Sean McGivern @smcgivern commented Apr 05, 2017
    Master

    @felipe_artur I've merged the backport MR, one last request!

    @felipe_artur I've merged the backport MR, one last request!
  • Sean McGivern @smcgivern

    assigned to @felipe_artur

    Apr 05, 2017

    assigned to @felipe_artur

    assigned to @felipe_artur
    Toggle commit list
  • Felipe Artur @felipe_artur

    added 1 commit

    • 065fa720 - Remove useless return burndown.rb

    Compare with previous version

    Apr 05, 2017

    added 1 commit

    • 065fa720 - Remove useless return burndown.rb

    Compare with previous version

    added 1 commit * 065fa720 - Remove useless return burndown.rb [Compare with previous version](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1540/diffs?diff_id=3751028&start_sha=163a87a7435af5ac0e303008bfb34d85e20f521e)
    Toggle commit list
  • Mike Greiling @mikegreiling

    resolved all discussions

    Apr 05, 2017

    resolved all discussions

    resolved all discussions
    Toggle commit list
  • Sean McGivern @smcgivern commented Apr 05, 2017
    Master

    @jschatz1 I'm happy with this, can you do the honours and merge please?

    @jschatz1 I'm happy with this, can you do the honours and merge please?
  • Sean McGivern @smcgivern

    assigned to @jschatz1

    Apr 05, 2017

    assigned to @jschatz1

    assigned to @jschatz1
    Toggle commit list
  • Jacob Schatz @jschatz1

    enabled an automatic merge when the pipeline for 065fa720 succeeds

    Apr 05, 2017

    enabled an automatic merge when the pipeline for 065fa720 succeeds

    enabled an automatic merge when the pipeline for 065fa7208564e53f34c10466f2268bdb93e98de5 succeeds
    Toggle commit list
  • Achilleas Pipinellis (🌴 back on April 30 🌴) @axil

    mentioned in issue #2092 (closed)

    Apr 05, 2017

    mentioned in issue #2092 (closed)

    mentioned in issue #2092
    Toggle commit list
  • Jacob Schatz @jschatz1

    canceled the automatic merge

    Apr 05, 2017

    canceled the automatic merge

    canceled the automatic merge
    Toggle commit list
  • Jacob Schatz @jschatz1

    enabled an automatic merge when the pipeline for 065fa720 succeeds

    Apr 05, 2017

    enabled an automatic merge when the pipeline for 065fa720 succeeds

    enabled an automatic merge when the pipeline for 065fa7208564e53f34c10466f2268bdb93e98de5 succeeds
    Toggle commit list
  • Jacob Schatz @jschatz1

    mentioned in commit 644abaf2

    Apr 05, 2017

    mentioned in commit 644abaf2

    mentioned in commit 644abaf20a970b3cc3f079807cebd46454ab2608
    Toggle commit list
  • Jacob Schatz @jschatz1

    merged

    Apr 05, 2017

    merged

    merged
    Toggle commit list
  • Mike Greiling @mikegreiling commented Apr 06, 2017
    Master

    yay 🎉

    yay :tada:
  • Pedro Moreira da Silva @pedroms commented Apr 06, 2017
    Developer

    Thanks to everyone involved, this is an awesome feature! 👏

    Thanks to everyone involved, this is an awesome feature! :clap:
  • Douwe Maan @DouweM

    mentioned in issue #2578 (closed)

    Jun 05, 2017

    mentioned in issue #2578 (closed)

    mentioned in issue #2578
    Toggle commit list
  • Write
  • Preview
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment
Jacob Schatz
Assignee
Jacob Schatz @jschatz1
Assign to
9.1
Milestone
9.1
Assign milestone
Time tracking
3
Labels
Deliverable frontend milestones
Assign labels
  • View project labels
Reference: gitlab-org/gitlab-ee!1540
×

Revert this merge request

This will create a new commit in order to revert the existing changes.

Switch branch
Cancel
A new branch will be created in your fork and a new merge request will be started.
×

Cherry-pick this merge request

Switch branch
Cancel
A new branch will be created in your fork and a new merge request will be started.