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

Update AL-Go Telemetry Module #1049

Merged
merged 43 commits into from
Aug 6, 2024
Merged

Update AL-Go Telemetry Module #1049

merged 43 commits into from
Aug 6, 2024

Conversation

aholstrup1
Copy link
Collaborator

  • Log telemetry without needing bccontainerhelper
  • Remove parentTelemetryScopeJson inputs to AL-Go actions
  • Add a bit more GitHub context to telemetry

@aholstrup1 aholstrup1 changed the title [Draft] Update AL-Go Telemetry Module Update AL-Go Telemetry Module May 1, 2024
@aholstrup1 aholstrup1 marked this pull request as ready for review May 1, 2024 09:46
@aholstrup1 aholstrup1 requested a review from a team as a code owner May 1, 2024 09:46
mazhelez
mazhelez reviewed May 1, 2024
View reviewed changes
Copy link
Collaborator

@mazhelez mazhelez left a comment

Choose a reason for hiding this comment

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

Initial review. See comments.

There are also some TODOs that need to be addressed.

aholstrup1 and others added 4 commits May 2, 2024 14:13
@aholstrup1
Casing
e32a67e
@aholstrup1
Invoke-Action wrapper (#32)
9f76e3a 
* Action wrapper

* Invoke-Command

* Try on other actions

* Remove telemetry module

* Wrappers

* Remove todos

* formatting

* test new structure

* Update wrapper

* Update YAML test
@aholstrup1
Merge branch 'main' of https://github.com/microsoft/al-go into aholst…
902f8aa 
…rup/telemetrymodule
@aholstrup1
ALGO Branch updates (#33)
c9ba11c 
* Update

* Remove logging
github-advanced-security[bot]
github-advanced-security bot found potential problems May 3, 2024
View reviewed changes
mazhelez
mazhelez reviewed May 6, 2024
View reviewed changes
mazhelez
mazhelez previously approved these changes May 6, 2024
View reviewed changes
Copy link
Collaborator

@mazhelez mazhelez left a comment

Choose a reason for hiding this comment

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

Looks okay to me.

I think GitHub doesn't do well in presenting changes in big files, but please verify there were no merging issues.

freddydk
freddydk previously approved these changes May 17, 2024
View reviewed changes
Copy link
Contributor

@freddydk freddydk left a comment

Choose a reason for hiding this comment

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

Only two questions really, else it looks good.

mazhelez
mazhelez previously approved these changes May 17, 2024
View reviewed changes
mazhelez
mazhelez reviewed Aug 1, 2024
View reviewed changes
Copy link
Collaborator

@mazhelez mazhelez left a comment

Choose a reason for hiding this comment

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

Left some comments. Nothing major, but I think one more iteration might be needed.

mazhelez
mazhelez previously approved these changes Aug 5, 2024
View reviewed changes
freddydk
freddydk requested changes Aug 5, 2024
View reviewed changes
Copy link
Contributor

@freddydk freddydk left a comment

Choose a reason for hiding this comment

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

Looks good - only one change requested

freddydk
freddydk previously approved these changes Aug 5, 2024
View reviewed changes
@aholstrup1 aholstrup1 enabled auto-merge (squash) August 6, 2024 11:01
@aholstrup1 aholstrup1 merged commit 8ec6169 into microsoft:main Aug 6, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@freddydk freddydk freddydk approved these changes

@mazhelez mazhelez mazhelez 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.
3 participants
@aholstrup1 @freddydk @mazhelez