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

feat: add alwaysPrintInCI option #76

Merged
merged 3 commits into from
Dec 16, 2024
Merged

feat: add alwaysPrintInCI option #76

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c7beaf4
fix(ci-output): properly cache dynamic info frames
cristiand391 Dec 12, 2024
d601d37
feat: add `alwaysPrintInCI` option
cristiand391 Dec 13, 2024
4e407cc
chore: update sf-deploy example
cristiand391 Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/multi-stage-output.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ class CIMultiStageOutput<T extends Record<string, unknown>> {

const formattedData = info.get ? info.get(this.data as T) : undefined
if (!formattedData) continue
const key = info.type === 'message' ? formattedData : info.label
const key = info.type === 'message' ? formattedData : `${info.label}: ${formattedData}`
Copy link
Member Author

Choose a reason for hiding this comment

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

key will include dynamic data so new output based on updated data will always be printed

Copy link
Contributor

Choose a reason for hiding this comment

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

The key is only the label for dynamic data because, otherwise, it will print on every single update - which is what we wanted to avoid

Run OCLIF_CI_UPDATE_FREQUENCY_MS=500 CI=true tsx examples/sf-specific/deploy.ts before and after your change to get an idea of the difference

Copy link
Contributor

Choose a reason for hiding this comment

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

Just thinking out loud here: I wonder if our best option is to add a new option to InfoBlock that will tell the CI output to always print - regardless of it the update timeout has been met or not.

So something like this:

{
  label: 'Successful',
  get: (data): string | undefined => {
     // formatting stuff here
  }
},
{
  label: 'Failed',
  alwaysPrintInCI: true,
  get: (data): string | undefined => {
     // formatting stuff here
  }
}

That way customers always get updates Failed but only intermittently get updates to Successful

Copy link
Member Author

Choose a reason for hiding this comment

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

Run OCLIF_CI_UPDATE_FREQUENCY_MS=500 CI=true tsx examples/sf-specific/deploy.ts before and after your change to get an idea of the difference

ohh I see now.

Just thinking out loud here: I wonder if our best option is to add a new option to InfoBlock that will tell the CI output to always print - regardless of it the update timeout has been met or not.

I like that option, I was struggling in PDR trying to find why updates were missing til I found about the throttle env affected it (which lead me to make this change first).
With the new option, the InfoBlock getters would need explicitly handle possible duplicates like this:
https://github.com/salesforcecli/plugin-deploy-retrieve/blob/fcc83af585eca9fc8f47db48a827a645e7fed116/src/utils/deployStages.ts#L161C1-L168C16

I'll add it in a bit 👍🏼

const str = info.type === 'message' ? formattedData : `${info.label}: ${formattedData}`

const lastUpdateTime = this.lastUpdateByInfo.get(key)
Expand Down
Loading