On Tue, 11 Jun 2024 13:34:24 GMT, Sonia Zaldana Calles <szald...@openjdk.org> 
wrote:

>> Hi all, 
>> 
>> This PR addresses [8333685](https://bugs.openjdk.org/browse/JDK-8333685). 
>> 
>> I have added the following enhancements: 
>> - Removed uses of `fgrep` and `egrep` with `grep -F` and `grep -E` 
>> respectively. 
>> - Altered default behaviour to limit the processed changesets to those in 
>> the current branch and the current year. 
>> - Enabled an option to modify all changesets in the year if desired (this 
>> was the previous default behaviour). 
>> - Added named parameters and enabled `--help`.
>> - Removed mercurial support. 
>> - Fixed a bug where copyright headers that didn't have a comma following the 
>> initial year of creation were not getting picked up. For example, 
>> [here](https://github.com/openjdk/jdk/blob/512b2b4f141f9a202984150b0427372e1a409a50/src/hotspot/cpu/zero/copy_zero.hpp#L3).
>>  
>> - Fixed a bug where copyright headers missing the copyright symbol (c) were 
>> not getting picked up. Refer to the example above as well. 
>> 
>> Thanks, 
>> Sonia
>
> Sonia Zaldana Calles has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Fixing optional group to work with bsd as well

A lot better, but not fully there yet. Can you make it so that it also picks up 
SAP's copyrights when they specify multiple years? They are weird in that the 
last year does not have a trailing comma, e.g.: 

* Copyright (c) 2020, 2023 SAP SE. All rights reserved.

make/scripts/update_copyright_year.sh line 70:

> 68:   echo "y     Specifies the copyright year. Set to current year by 
> default."
> 69:   echo "h     Print this help."
> 70:   echo "f     Updates all change sets in a full year."

- Please specify the leading dash on all options
- I would print "y" after "f" and write something along these lines "If -f is 
specified, defines the year of changes for which to modify the copyright 
(current year if omitted)"

make/scripts/update_copyright_year.sh line 103:

> 101: [ -d "${this_script_dir}/../../.git" ] && git_found=true
> 102: if [ "$git_found" != "true" ]; then
> 103:   echo "Error: could not auto-detect git as the version control system"

Pre-existing. The error message is confusing (I first tried your script from a 
different location). Better would be adding something like this:

"Please execute script from within make/scripts"

-------------

Changes requested by stuefe (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19605#pullrequestreview-2112081245
PR Review Comment: https://git.openjdk.org/jdk/pull/19605#discussion_r1635897295
PR Review Comment: https://git.openjdk.org/jdk/pull/19605#discussion_r1635901072

Reply via email to