Skip to content
This repository was archived by the owner on Jul 12, 2022. It is now read-only.

Fix timer os metric build formula watch #601

Conversation

brunats
Copy link
Contributor

@brunats brunats commented Oct 7, 2020

- What I did
Add stop condition to the hit build formula --watch command. Now the process can be completed correctly when the user presses 'Ctrl + c'.

This corrects the timing of sending metrics that is next to the process stop.

stop

- How to verify it

- Description for the changelog

Fix timer os metric build formula watch

Signed-off-by: Bruna Tavares <silvatavares.bruna@gmail.com>
@brunasilvazup brunasilvazup added ✨ feature Suggest a new feature or enhancement to the Ritchie project 🚧 WIP Work in Progress labels Oct 8, 2020
Signed-off-by: Bruna Tavares <silvatavares.bruna@gmail.com>
…added-metrics-specific-to-rit-build-watch

Signed-off-by: Bruna Tavares <silvatavares.bruna@gmail.com>
@brunasilvazup brunasilvazup linked an issue Oct 8, 2020 that may be closed by this pull request
@brunasilvazup brunasilvazup marked this pull request as ready for review October 8, 2020 19:50
@brunasilvazup brunasilvazup added ✔️ ready-for-review ready for review and removed 🚧 WIP Work in Progress labels Oct 8, 2020
prompt.Info(watchText)

startTime := time.Now().Second()
w.sendMetric(float64(startTime))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious, where is the start time being collected now? And how does it differ a start from an end? sendMetric interprets it on the 2nd call?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I also had this question, I just changed the location, but I didn't check the difference.

In the main it uses a function to calculate the difference.
And checking now I saw that leaving the send metric here he sends twice.

In the case of the main send metric it uses save the start time and get the time again at the end of the execution.

Signed-off-by: Bruna Tavares <silvatavares.bruna@gmail.com>
@@ -54,8 +57,22 @@ func New(
}
}

func (w *WatchManager) closeWatch() {
currentTime := time.Now().Second()
w.sendMetric(float64(currentTime))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this asynchronous or should the stopping text come before it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sendMetric is asynchronous (:

Copy link
Contributor

@henriquemoraeszup henriquemoraeszup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment but lgtm

@codecov-io
Copy link

Codecov Report

Merging #601 into master will decrease coverage by 0.20%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #601      +/-   ##
==========================================
- Coverage   80.18%   79.97%   -0.21%     
==========================================
  Files          98       98              
  Lines        3265     3246      -19     
==========================================
- Hits         2618     2596      -22     
- Misses        462      465       +3     
  Partials      185      185              
Impacted Files Coverage Δ
pkg/formula/watcher/watcher.go 76.19% <100.00%> (-12.34%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c704f4b...b05f8d6. Read the comment docs.

@brunasilvazup
Copy link
Contributor

brunasilvazup commented Oct 15, 2020

/merge qa

@ritchie-bot
Copy link
Contributor

ritchie-bot bot commented Oct 15, 2020

👌 Merged branch feature/added-metrics-specific-to-rit-build-watch into qa

@lucasdittrichzup
Copy link
Contributor

🚀

@brunasilvazup brunasilvazup merged commit edc6ea9 into ZupIT:master Oct 16, 2020
brunonmelo pushed a commit to brunonmelo/ritchie-cli that referenced this pull request Nov 12, 2020
* Added channel to listen signals

Signed-off-by: Bruna Tavares <silvatavares.bruna@gmail.com>

* Added signal to test

Signed-off-by: Bruna Tavares <silvatavares.bruna@gmail.com>

* Improves text and corrects metric delivery

Signed-off-by: Bruna Tavares <silvatavares.bruna@gmail.com>
Signed-off-by: Bruno N. Melo <brunonobrega.melo@gmail.com>
brunonmelo pushed a commit to brunonmelo/ritchie-cli that referenced this pull request Nov 12, 2020
* Added channel to listen signals

Signed-off-by: Bruna Tavares <silvatavares.bruna@gmail.com>

* Added signal to test

Signed-off-by: Bruna Tavares <silvatavares.bruna@gmail.com>

* Improves text and corrects metric delivery

Signed-off-by: Bruna Tavares <silvatavares.bruna@gmail.com>
Signed-off-by: Bruno N. Melo <brunonobrega.melo@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
✔️ ready-for-review ready for review ✨ feature Suggest a new feature or enhancement to the Ritchie project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collect the execution time of the --watch flag
6 participants