Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: remove error from analytics #3802

Merged
merged 4 commits into from
Dec 4, 2023
Merged

refactor: remove error from analytics #3802

merged 4 commits into from
Dec 4, 2023

Conversation

Pantani
Copy link
Collaborator

@Pantani Pantani commented Dec 4, 2023 •
edited
Loading

part of #3800

@Pantani Pantani self-assigned this Dec 4, 2023
salmad3
salmad3 previously approved these changes Dec 4, 2023
View reviewed changes
@salmad3 salmad3 added the skip-changelog Don't check changelog for new entries label Dec 4, 2023
julienrbrt
julienrbrt requested changes Dec 4, 2023
View reviewed changes
Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

I don't think we need the option and the panic recovery

ilgooz
ilgooz requested changes Dec 4, 2023
View reviewed changes
Copy link
Member

@ilgooz ilgooz left a comment

Choose a reason for hiding this comment

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

Can we also remove flags and arguments, only keeping the command in full?

@Pantani
Copy link
Collaborator Author

Pantani commented Dec 4, 2023

Can we also remove flags and arguments, only keeping the command in full?

yes, sure, this PR is only to remove the error, I'm doing in another PR

@Pantani Pantani requested a review from ilgooz December 4, 2023 16:31
@Pantani Pantani requested review from salmad3 and julienrbrt December 4, 2023 16:31
@codecov Codecov
Copy link

codecov bot commented Dec 4, 2023

Codecov Report

Merging #3802 (3b98086) into main (ab3f5c1) will decrease coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3802      +/-   ##
==========================================
- Coverage   24.29%   24.28%   -0.01%     
==========================================
  Files         292      292              
  Lines       24428    24428              
==========================================
- Hits         5934     5932       -2     
- Misses      17956    17957       +1     
- Partials      538      539       +1

see 1 file with indirect coverage changes

@julienrbrt julienrbrt changed the title feat: remove error from analytics refactor: remove error from analytics Dec 4, 2023
julienrbrt
julienrbrt approved these changes Dec 4, 2023
View reviewed changes
Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

lgtm

@ilgooz ilgooz merged commit 69db6cc into main Dec 4, 2023
42 of 43 checks passed
@ilgooz ilgooz deleted the feat/remove-error-from-analytics branch December 4, 2023 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julienrbrt julienrbrt julienrbrt approved these changes

@jeronimoalbi jeronimoalbi Awaiting requested review from jeronimoalbi jeronimoalbi is a code owner

@Ehsan-saradar Ehsan-saradar Awaiting requested review from Ehsan-saradar

@ilgooz ilgooz Awaiting requested review from ilgooz ilgooz is a code owner

@salmad3 salmad3 Awaiting requested review from salmad3

Assignees

@Pantani Pantani

Labels
component:ci CI/CD workflow and automated jobs. component:cmd component:configs component:packages skip-changelog Don't check changelog for new entries type:internal
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Pantani @ilgooz @salmad3 @julienrbrt