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

fix: adding warning when pushing monitors from yaml files #977

Merged
merged 6 commits into from
Nov 1, 2024
Merged

fix: adding warning when pushing monitors from yaml files #977

merged 6 commits into from
Nov 1, 2024

Conversation

Spoonsk
Copy link
Contributor

@Spoonsk Spoonsk commented Oct 31, 2024 •
edited by vigneshshanmugam
Loading

@cla-checker-service CLA Checker Service
Copy link

cla-checker-service bot commented Oct 31, 2024 •
edited
Loading

💚 CLA has been signed

@Spoonsk
Copy link
Contributor Author

Spoonsk commented Oct 31, 2024

❌ Author of the following commits did not sign a Contributor Agreement: 8e1d65c, f855a3c

Please, read and sign the above mentioned agreement if you want to contribute to this project

I've since signed the agreement. Let me know if you need anything else here.

vigneshshanmugam
vigneshshanmugam reviewed Oct 31, 2024
View reviewed changes
src/push/monitor.ts Outdated Show resolved Hide resolved
src/push/monitor.ts Outdated Show resolved Hide resolved
@vigneshshanmugam vigneshshanmugam changed the title Adding warnings to npx push fix: adding warning when pushing monitors from yaml files Nov 1, 2024
@vigneshshanmugam
Copy link
Member

@Spoonsk Thank you for the contribution :)

@vigneshshanmugam
Copy link
Member

@Spoonsk Can you sign the CLA again? Thanks.

@Spoonsk
Copy link
Contributor Author

Spoonsk commented Nov 1, 2024

@Spoonsk Can you sign the CLA again? Thanks.

I filled out the CLA again with my github name and we are still seeing the CLA failure. Is there something we need to do to retrigger the CLA evaluation?

@shahzad31
Copy link
Contributor

@Spoonsk i see commits from 2 users names, Joe Gast and you, maybe Joe Gast also need to sign?

@JosephGast
Copy link

@Spoonsk i see commits from 2 users names, Joe Gast and you, maybe Joe Gast also need to sign?

I just signed.

@Spoonsk
Copy link
Contributor Author

Spoonsk commented Nov 1, 2024

I see the issue. We have some commits on here that were created by a user with an email that isn't associated with a github account. I will try to re-author those commits on this Branch, if that doesn't work. Then I may need to create another branch with another PR and we can abandon this one.

Spoonsk and others added 6 commits November 1, 2024 13:32
@Spoonsk
package.json - added dependency 'utf-8-validate'
3b1e726
@Spoonsk
monitor.ts - Added validation and warnings
f02fd21 
- Validation/warning prior to UTF-8 decoding of files
- warning when a 'browser' type monitor is ignored
- removing unused variable (causing linter error)
@Spoonsk @vigneshshanmugam
Update src/push/monitor.ts
ca366f4 
changing warning text

Co-authored-by: Vignesh Shanmugam <[email protected]>
@Spoonsk
src/push/monitor.ts - using node standard libarary to test utf-8 enco…
196c207 
…ding

- updated types@node to be more in line with current version
@Spoonsk @vigneshshanmugam
Update src/push/monitor.ts - updating warning message.
3e952ff 
Co-authored-by: Vignesh Shanmugam <[email protected]>
@vigneshshanmugam @Spoonsk
fix node type and message
74933d0
@Spoonsk Spoonsk force-pushed the adding-warnings-to-npx-push branch from 87d34bc to 74933d0 Compare November 1, 2024 20:35
@Spoonsk
Copy link
Contributor Author

Spoonsk commented Nov 1, 2024

There we go. I was able to re-author the Joe Gast commits. Should CLA should be happy now.

vigneshshanmugam
vigneshshanmugam approved these changes Nov 1, 2024
View reviewed changes
Copy link
Member

@vigneshshanmugam vigneshshanmugam left a comment

Choose a reason for hiding this comment

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

Thank you @Spoonsk

justinkambic
justinkambic approved these changes Nov 1, 2024
View reviewed changes
Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

LGTM!!

@vigneshshanmugam vigneshshanmugam merged commit cf9d891 into elastic:main Nov 1, 2024
8 checks passed
@Spoonsk Spoonsk deleted the adding-warnings-to-npx-push branch November 1, 2024 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinkambic justinkambic justinkambic approved these changes

@vigneshshanmugam vigneshshanmugam vigneshshanmugam approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add warnings for silent failures during lightweight synthetics pushes
5 participants
@Spoonsk @vigneshshanmugam @shahzad31 @JosephGast @justinkambic