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.

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

Concept: Commands and Arguments #699

Merged
merged 6 commits into from
Aug 14, 2024
Merged

Conversation

glennj
Copy link
Contributor

@glennj glennj commented Jul 14, 2024

The first concept in the syllabus.

@glennj glennj marked this pull request as draft July 14, 2024 18:20
concepts/README.md Outdated Show resolved Hide resolved
concepts/README.md Outdated Show resolved Hide resolved
concepts/README.md Show resolved Hide resolved
concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
@glennj
Copy link
Contributor Author

glennj commented Jul 15, 2024

Thank you for the details comments. I'll process them anon.

Taking a step back, are there any topics in there that don't need to be in the first concept document? Is it too wordy, too detailed?

@IsaacG
Copy link
Member

IsaacG commented Jul 18, 2024

Thank you for the details comments. I'll process them anon.

Taking a step back, are there any topics in there that don't need to be in the first concept document? Is it too wordy, too detailed?

I think you did a good job here. It's a bit wordy for my tastes, but I also prefer reading terse man pages. The topic selection seems reasonable and the order of the material seems to make sense to me.

We could ask for a volunteer with less bash experience to do a read through of the contents once the PR is ready (or after the syllabus is coming together but before a release) to give us an "outsider" perspective.

concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
@glennj
Copy link
Contributor Author

glennj commented Jul 19, 2024

Reading it over, I'm thinking the whole Strings section can be removed.

I'd like to trim down the paragraphs above Strings, but quoting is super important.

Removed "strings" section.
Removed "review" section in the middle of the document.
Removed note about Windows editors and line endings.
Trimmed some of the fat.
Lowercased "bash" except at start of sentences.
@glennj glennj requested review from kotp and IsaacG July 28, 2024 16:28
@kotp
Copy link
Member

kotp commented Jul 28, 2024

@glennj is this still in Draft? Or rather, it is, but should it be?

@glennj glennj marked this pull request as ready for review July 28, 2024 19:21
@glennj
Copy link
Contributor Author

glennj commented Jul 28, 2024

Right, getting close.

Copy link
Member

@IsaacG IsaacG left a comment

Choose a reason for hiding this comment

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

I think this is a solid intro doc.

concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
concepts/commands-and-arguments/introduction.md Outdated Show resolved Hide resolved
$ [ -f file ]
```

(We see a lot of people who are confused by this behavior; they think that they can omit the whitespace between `[` and its arguments, so we need to present this particular example early.)
Copy link
Member

Choose a reason for hiding this comment

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

Note, Wooledge is a Wiki. Anything we copy/paste then change, that change can also be applied upstream.

@glennj
Copy link
Contributor Author

glennj commented Aug 2, 2024

@IsaacG @kotp A new commit, addressing Isaac's last round of suggestions.

I added about.md, identical to introduction.md.

I threw in an example about < as a string operator in [[ vs [.

This is probably done. When I have your thoughts, I'll throw it open to the forum for suggestions.

Copy link
Member

@kotp kotp left a comment

Choose a reason for hiding this comment

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

Suggested to remove the extra blank line (in both files).

fi
# => glennj is in the first half of the alphabet
```

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

fi
# => glennj is in the first half of the alphabet
```

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

@kotp
Copy link
Member

kotp commented Aug 3, 2024

The discussion on the forums is here. Maybe edit the first message with the link?

@glennj glennj merged commit 0198368 into exercism:main Aug 14, 2024
2 checks passed
@glennj glennj deleted the commands-concept branch August 14, 2024 17:50
@glennj
Copy link
Contributor Author

glennj commented Aug 22, 2024

@glennj glennj added x:module/concept Work on Concepts x:type/content Work on content (e.g. exercises, concepts) labels Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:module/concept Work on Concepts x:type/content Work on content (e.g. exercises, concepts)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants