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

Init CHANGELOG.md #33

Merged
merged 1 commit into from
Dec 8, 2024
Merged

Init CHANGELOG.md #33

merged 1 commit into from
Dec 8, 2024

Conversation

samueloph
Copy link
Member

No description provided.

@samueloph samueloph force-pushed the samueloph/CHANGELOG.md branch 2 times, most recently from 8abdf10 to 8821818 Compare December 7, 2024 19:45
Copy link
Collaborator

@sergiodj sergiodj left a comment

Choose a reason for hiding this comment

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

Thanks, @samueloph.

I left some comments and requests for change, but the file is looking good.

CHANGELOG.md Outdated
* First release to be shipped on Debian.

## [v2024-06-26]
* Second release.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not that it matters much, but would you like to add some information on what changed here?

CHANGELOG.md Outdated
* Update LICENSE file with new contributors.

## [v2024-07-07]
* Drop getotp usage, non-linux environments are supported now.
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • s/getotp/getopt/

  • Use backticks on getopt.

  • I will personally use "GNU/Linux", but it's OK if you don't want to. However, I believe we should at least capitalize the name "Linux" here.

CHANGELOG.md Outdated

## [v2024-07-07]
* Drop getotp usage, non-linux environments are supported now.
* Replace "-o/--opts=" parameters with "--curl-options/--curl-options=".
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this file is explicitly written in Markdown, we should use backticks here and elsewhere.

CHANGELOG.md Outdated
This alternative is more descriptive and it does not coincide with any of curl's parameters.
* Stop auto-resuming downloads and don't overwrite files instead by default.
Safer alternative as otherwise curl can corrupt a file if the name clashes and the size of the existing one is smaller.
One can easily change that behavior with '--curl-options="--continue-at -'.
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a mix of single and double quotes here (but we should use backticks).

CHANGELOG.md Outdated
Safer alternative as otherwise curl can corrupt a file if the name clashes and the size of the existing one is smaller.
One can easily change that behavior with '--curl-options="--continue-at -'.
* New --dry-run option: just print what would be invoked.
* Choose https as a default protocol, in case there's none in the URL.
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/https/HTTPS/.

CHANGELOG.md Outdated
* Choose https as a default protocol, in case there's none in the URL.
* Disable curl's URL globbing parser so {} and [] characters in URLs are not treated specially.
* Implement support for '--'.
* Implement a -V and --version options.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Implement a -V and --version options.
* Implement `-V/--version` options.

CHANGELOG.md Outdated
- New version: 2024.07.10
* Support older curl releases, minimum required version is now 7.46.0:
- Only set --no-clobber if curl is 7.83 or newer
- Only set --parallel if curl is 7.66 or newer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing period at the end of this sentence and the previous one.

CHANGELOG.md Outdated
* Support older curl releases, minimum required version is now 7.46.0:
- Only set --no-clobber if curl is 7.83 or newer
- Only set --parallel if curl is 7.66 or newer
* Set --fail on curl, in order to return errors instead of saving as
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Set --fail on curl, in order to return errors instead of saving as
* Set `--fail` when invoking curl, in order to display possible errors instead of saving them as

CHANGELOG.md Outdated
output files.
* Add more tests.
* Remove the need for GNU coreutils' realpath for tests.
* wcurl.1: Update manpage with links to Github and Debian's Salsa.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This format of file: what changed isn't used anywhere else, so I think we shouldn't use it here either.

CHANGELOG.md Outdated
# Changelog

## [UNRELEASED]
* Default to index.html as filename if non can be parsed from the URL.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there's some typo here...? I don't fully understand the sentence.

@samueloph samueloph force-pushed the samueloph/CHANGELOG.md branch from 8821818 to 4a68168 Compare December 8, 2024 18:54
@samueloph
Copy link
Member Author

Thanks, @samueloph.

I left some comments and requests for change, but the file is looking good.

Thank you @sergiodj, I believe I have addressed all of them with the latest changes.

@samueloph samueloph requested a review from sergiodj December 8, 2024 19:07
Copy link
Collaborator

@sergiodj sergiodj left a comment

Choose a reason for hiding this comment

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

Thanks, @samueloph.

I'm leaving a second round of suggestions that are ready to be applied. Feel free to merge the PR afterwards.

CHANGELOG.md Outdated

## [UNRELEASED]
* New parameter `-o|-O|--output|output=` which allows the user to choose the output filename.
* Default to "index.html" as filename if none can be inferred from the URL.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Default to "index.html" as filename if none can be inferred from the URL.
* Default to `index.html` as filename if none can be inferred from the URL.

CHANGELOG.md Outdated
Comment on lines 27 to 28
- Previous version: 2024-07-07
- New version: 2024.07.10
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- Previous version: 2024-07-07
- New version: 2024.07.10
- Previous version: `2024-07-07`
- New version: `2024.07.10`

CHANGELOG.md Outdated
* Set `--fail` when invoking curl, in order to display possible errors instead of saving them as
output files.
* Add more tests.
* Remove the need for GNU coreutils' realpath for tests.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Remove the need for GNU coreutils' realpath for tests.
* Remove the need for GNU coreutils' `realpath` for tests.

CHANGELOG.md Outdated
* New `--dry-run` option: just print what would be invoked.
* Choose HTTPS as a default protocol, in case there's none in the URL.
* Disable curl's URL globbing parser so `{}` and `[]` characters in URLs are not treated specially.
* Implement support for `--`'.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Implement support for `--`'.
* Implement support for `--`.

CHANGELOG.md Outdated
* Only set `--parallel` if there's more than one URL.
* Fix manpage typo.
* Update COPYRIGHT and AUTHORS in manpage.
* Rewrite wcurl to remove bash dependency, it's now a POSIX shellscript.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Rewrite wcurl to remove bash dependency, it's now a POSIX shellscript.
* Rewrite wcurl to remove bash dependency, it's now a POSIX shell script.

CHANGELOG.md Outdated
* Add LICENSE.

## [v2024-06-26]
* Simplify`--help` output.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Simplify`--help` output.
* Simplify `--help` output.

CHANGELOG.md Outdated

## [v2024-07-07]
* Drop `getopt` usage, non-GNU/Linux environments are supported now.
* Replace `-o/--opts=` parameters with `--curl-options/--curl-options=`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Replace `-o/--opts=` parameters with `--curl-options/--curl-options=`.
* Replace `-o`/`--opts=` parameters with `--curl-options`/`--curl-options=`.

CHANGELOG.md Outdated
* Choose HTTPS as a default protocol, in case there's none in the URL.
* Disable curl's URL globbing parser so `{}` and `[]` characters in URLs are not treated specially.
* Implement support for `--`'.
* Implement `-V/--version` options.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Implement `-V/--version` options.
* Implement `-V`/`--version` options.

@samueloph samueloph force-pushed the samueloph/CHANGELOG.md branch from 4a68168 to 09be7a5 Compare December 8, 2024 22:21
@samueloph samueloph requested a review from sergiodj December 8, 2024 22:31
@samueloph samueloph force-pushed the samueloph/CHANGELOG.md branch from 09be7a5 to b653a92 Compare December 8, 2024 22:54
@samueloph samueloph merged commit 340ac12 into main Dec 8, 2024
1 check passed
@samueloph samueloph deleted the samueloph/CHANGELOG.md branch December 8, 2024 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants