-
Notifications
You must be signed in to change notification settings - Fork 54
NEW @W-16891765@ Config action mentions outfile #1649
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.
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
Conversation
| # common.summary-header | ||
|
|
||
| Summary | ||
|
|
||
| # common.logfile-location | ||
|
|
||
| Additional log information written to: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied from run-summary-viewer.md. Named common.blah to allow for porting the existing RunSummaryViewer implementation to this new style.
|
|
||
| # config-action.no-outfiles | ||
|
|
||
| No output file was specified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Analogous to run-summary-viewer.md's message "No results files were specified."
|
|
||
| No output file was specified. | ||
|
|
||
| # config-action.outfile-location |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Analogous to run-summary-viewer.md's message "Results written to:".
| import {toStyledHeader, indent} from '../utils/StylingUtil'; | ||
| import {BundleName, getMessage} from '../messages'; | ||
|
|
||
| abstract class AbstractActionSummaryViewer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's only one child class right now, but the parent class will be helpful for porting the Run Summary Viewer to the new style.
| } | ||
|
|
||
| public async write(model: ConfigModel): Promise<void> { | ||
| public async write(model: ConfigModel): Promise<boolean> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this method isn't guaranteed to actually write anything, it now returns a boolean to indicate whether it did.
| import {ConfigAction, ConfigDependencies, ConfigInput} from '../../../src/lib/actions/ConfigAction'; | ||
| import {AnnotatedConfigModel} from '../../../src/lib/models/ConfigModel'; | ||
| import {ConfigStyledYamlViewer} from '../../../lib/lib/viewers/ConfigViewer'; | ||
| import {ConfigStyledYamlViewer} from '../../../src/lib/viewers/ConfigViewer'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import path was wrong here.
| import {AnnotatedConfigModel} from '../../../src/lib/models/ConfigModel'; | ||
| import {ConfigStyledYamlViewer} from '../../../lib/lib/viewers/ConfigViewer'; | ||
| import {ConfigStyledYamlViewer} from '../../../src/lib/viewers/ConfigViewer'; | ||
| import {ConfigActionSummaryViewer} from '../../../src/lib/viewers/ActionSummaryViewer'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing through ConfigAction using the actual implementation, instead of using an Interface/Stub setup.
|
|
||
| // ==== OUTPUT PROCESSING ==== | ||
| const displayEvents = spyDisplay.getDisplayEvents(); | ||
| expect(displayEvents).toHaveLength(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Array length is now variable, so we're not checking for an exact length anymore.
dfec6ef to
9407177
Compare
messages/action-summary-viewer.md
Outdated
|
|
||
| # config-action.outfile-location | ||
|
|
||
| Config written to: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Config written to: | |
| Configuration written to: |
jshackell-sfdc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, but see my suggestion.
0c51e6e to
b03d267
Compare
| this.toYamlEngineOverrides() + '\n' + | ||
| '\n' + | ||
| this.toYamlSectionHeadingComment(getMessage(BundleName.ConfigModel, 'template.common.end-of-config')) + '\n'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this end of config message might end up in the file as well. I was hoping it would just be for the terminal output only.
No description provided.