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

Make the README less misleading by putting some reasonable defaults in (instead of empty strings '') #302

Merged
merged 6 commits into from
Dec 16, 2024
Merged

Make the README less misleading by putting some reasonable defaults in (instead of empty strings '') #302

merged 6 commits into from
Dec 16, 2024

Conversation

DilumAluthge
Copy link
Member

No description provided.

@DilumAluthge DilumAluthge requested a review from a team as a code owner November 18, 2024 18:43
@DilumAluthge DilumAluthge mentioned this pull request Nov 18, 2024
Merged
@IanButterworth
Copy link
Member

Probably do the same for the version too?

@DilumAluthge
Copy link
Member Author

Probably do the same for the version too?

Yeah. But what do we want to put in there?

@IanButterworth
Copy link
Member

The default? "1" note the "'s

@DilumAluthge DilumAluthge requested review from IanButterworth and removed request for IanButterworth November 18, 2024 18:54
IanButterworth
IanButterworth previously approved these changes Nov 18, 2024
View reviewed changes
vtjnash
vtjnash reviewed Nov 18, 2024
View reviewed changes
README.md
Comment on lines 71 to 72
#
# Default: false
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#
# Default: false

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm. I'm a bit conflicted on this. I don't know if the following line (show-versioninfo: 'false') makes it sufficiently clear that false is the default value. I think I'd probably lean towards keeping the line that clarifies that false is the default value.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md
Comment on lines 42 to 43
#
# Default: '1'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#
# Default: '1'

DilumAluthge and others added 2 commits November 18, 2024 14:31
@DilumAluthge
Copy link
Member Author

@IanButterworth Any idea why those two CI checks are showing up as "cancelled".

@DilumAluthge
Copy link
Member Author

Hmmm. Those two jobs keep showing up as canceled, and thus I cannot merge this PR.

@IanButterworth
Copy link
Member

I guess it's actions/runner-images#10721 because they're macos-12

@DilumAluthge
Copy link
Member Author

I see. I will remove those two macos-12 jobs from the list of required status checks.

@DilumAluthge DilumAluthge merged commit 887a1b8 into master Dec 16, 2024
68 of 70 checks passed
@DilumAluthge DilumAluthge deleted the dpa/readme branch December 16, 2024 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IanButterworth IanButterworth IanButterworth approved these changes

@vtjnash vtjnash vtjnash 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
@DilumAluthge @IanButterworth @vtjnash