Skip to content

Commit

Permalink
commit.hb: stop requiring a ChangeLog.txt entry for commits
Browse files Browse the repository at this point in the history
Editing ChangeLog.txt means to manually maintain a modification history
in timestamp order. This isn't practical with the Git model, and blocks
handling patches, including PRs. It served well in a the linear
development model of CVS and SVN, but no longer with Git. Git also
handles the commiter, author, and commit/authoring timestamps
automatically, making our existing manual method obsolete. Manually
keeping a duplicate copy of the commit message also makes working on PRs
much complicated, as we need to keep these in sync while making changes.
Existing method also made the one-liner commit messages 100%
meaningless, repeating the author/timestamp, which is visible there on
most UIs already.

Git allows to list the changelog with this command, dynamically:

`$ git log --pretty=medium --no-merges --date=iso --stat HEAD~50..HEAD`

I'm suggesting a new way of keeping history, which solves all these
issues and makes working with the codebase simpler and aligns Harbour
with most other projects over the internet:

In the top line, put a one-liner description of the changes, prefixed
with the module where the change happened, e.g.:
hbmk2: add support for Windows ARM64

This replaces the existing manual author/timestamp header.

Then an empty line, followed by the description of the changes, and/or
here we can use the existing format:

* filename1.ext:
  ! fix foo bar
* filename2.ext:
  + add foo baz

Full commit message:
```
hbmk2: add support for Windows ARM64

* filename1.ext:
  ! fix foo bar
* filename2.ext:
  + add foo baz
```

Because the filenames are also redundant and shown by Git, I suggest
replacing the actual filenames with module names or other,
less-specific, but descriptive, short and recognizable pointer:

Further streamlined commit message:
```
hbmk2: add support for Windows ARM64

- fix foo bar
- add foo baz

Ref: <commit> #<pr>
Fixes #<issue>
Closes #<pr>
```

This also drop the `!`, `+`, `%`, `*`, `;` list marker in favour of
`-`. The text describes the nature of the change in more universal way
anyway.

I suggest wrapping the message body at 72 characters. Using Markdown
markup is fine. Adding pointers to commit hashes and/or GitHub issues/
PRs helps to put a commit in context.

When moving around patches made in the old way, I recommend stripping
the hunk dealing with `ChangeLog.txt` to ease the process. Manually
of e.g. with this command:

`$ filterdiff -p1 --exclude=ChangeLog.txt`
  • Loading branch information
vszakats committed Dec 22, 2023
1 parent 91d0b4b commit 0161a37
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 161 deletions.
22 changes: 18 additions & 4 deletions ChangeLog.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,21 @@
/* Encoding: UTF-8 (No BOM) Notation (in 5th column):
* Change ! Fix % Optimize + Add - Remove ; Comment
Entries may not always be in chronological/commit order.
See license at the end of file. */
/* Manual maintenance of ChangeLog.txt is deprecated. Please add a commit
message instead, following this format:
```
module: add new feature

* change this bit
- add this bit
! fix this bit
% optimize this bit
; comment, or just use `-` for any item.

Ref: <commit> #<pr>
Fixes #<issue>
Closes #<pr>
```

Use 'add'/'fix'/etc. Not 'added', 'adding' or 'adds'.
*/

2022-06-30 11:27 UTC Viktor Szakats
* contrib/hbcrypto/3rd/ed25519/ed25519.diff
Expand Down
163 changes: 6 additions & 157 deletions bin/commit.hb
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,7 @@ PROCEDURE Main()
LOCAL cLocalRoot
LOCAL aFiles
LOCAL aChanges
LOCAL cLog
LOCAL nStart, nEnd
LOCAL cMyName
LOCAL cLogName
LOCAL lWasChangeLog
LOCAL cEOL

IF "-c" $ cli_Options()
CheckFileList( iif( Empty( cli_Values() ),, cli_Values() ) )
Expand Down Expand Up @@ -73,78 +68,10 @@ PROCEDURE Main()
"-f" $ cli_Options() .OR. ;
"--force" $ cli_Options()

cLogName := FindChangeLog( cVCS )
IF cLogName == ""
OutStd( hb_ProgName() + ": " + "cannot find ChangeLog file" + hb_eol() )
ErrorLevel( 2 )
ENDIF

IF "--check-only" $ cli_Options() .OR. ;
"--prepare-commit" $ cli_Options()

IF hb_AScan( aFiles, hb_FNameNameExt( cLogName ),,, .T. ) == 0
OutStd( hb_ProgName() + ": " + hb_StrFormat( "%1$s not updated. Run '%2$s %3$s' and retry.", cLogName, hb_FNameNameExt( hbshell_ProgName() ), CommitScript() ) + hb_eol() )
ErrorLevel( 3 )
RETURN
ELSE
IF ! GitIsMerge( cVCSDir )
cLog := GetLastEntry( MemoRead( cLogName ), @nStart, @nEnd )
IF ! cLog == ""
IF "--prepare-commit" $ cli_Options() .AND. ! Empty( cli_Values() )
hb_MemoWrit( cli_Values()[ 1 ], EntryToCommitMsg( cLog ) + hb_MemoRead( cli_Values()[ 1 ] ) )
ELSE
hbshell_gtSelect()
#if 0
/* if clipboard already contains part of the entry, do not overwrite it */
IF ! hb_StrReplace( hb_gtInfo( HB_GTI_CLIPBOARDDATA ), Chr( 13 ) + Chr( 10 ) ) $ hb_StrReplace( cLog, Chr( 13 ) + Chr( 10 ) )
hb_gtInfo( HB_GTI_CLIPBOARDDATA, EntryToCommitMsg( cLog ) )
ENDIF
#endif
ENDIF
ENDIF
ENDIF
ENDIF

ELSEIF ! cLogName == ""

IF cVCS == "git"
cMyName := GitUser()
ELSE
IF ! GetEnv( _CONFIGENV_ ) == ""
cMyName := GetEnv( _CONFIGENV_ )
ELSEIF hb_vfExists( cLocalRoot + _CONFIGFIL_ )
cMyName := AllTrim( hb_MemoRead( cLocalRoot + _CONFIGFIL_ ) )
ELSE
cMyName := "Firstname Lastname (me example.org)"
ENDIF
ENDIF
IF ! "--check-only" $ cli_Options() .AND. ;
! "--prepare-commit" $ cli_Options()

//

cLog := MemoRead( cLogName )
cEOL := hb_StrEOL( cLog )

GetLastEntry( cLog, @nStart, @nEnd )

IF nStart > 0

/* Strip last entry if it's empty to avoid adding double entries */
IF IsLastEntryEmpty( SubStr( cLog, nStart, nEnd - nStart ), cLogName, @lWasChangeLog )
OutStd( hb_ProgName() + ": " + hb_StrFormat( "Updating last empty %1$s entry", cLogName ) + hb_eol() )
cLog := Left( cLog, nStart - 1 ) + SubStr( cLog, nEnd )
ELSE
lWasChangelog := .T.
ENDIF

cLog := ;
Left( cLog, nStart - 1 ) + ;
MakeEntry( aChanges, cMyName, cLogName, lWasChangeLog, cEOL ) + cEOL + ;
SubStr( cLog, nStart )
ELSE
cLog += cEOL + MakeEntry( aChanges, cMyName, cLogName, .T., cEOL )
ENDIF

hb_MemoWrit( cLogName, cLog )
hb_MemoWrit( cLogName := cLocalRoot + "_commit.txt", MakeEntry( aChanges ) )

OutStd( hb_ProgName() + ": " + hb_StrFormat( "Edit %1$s and commit", cLogName ) + hb_eol() )
#if 0
Expand Down Expand Up @@ -226,25 +153,6 @@ STATIC FUNCTION InstallHook( cDir, cHookName, cCommand )

RETURN hb_MemoWrit( cName, cFile + Chr( 10 ) + cCommand + Chr( 10 ) )

STATIC FUNCTION FindChangeLog( cVCS )

LOCAL cDir
LOCAL cLogName

IF hb_vfExists( cLogName := "ChangeLog.txt" ) .OR. ;
hb_vfExists( cLogName := "ChangeLog" )
RETURN cLogName
ENDIF

cDir := iif( cVCS == "git", GitLocalRoot(), "" )

IF hb_vfExists( cLogName := cDir + "ChangeLog.txt" ) .OR. ;
hb_vfExists( cLogName := cDir + "ChangeLog" )
RETURN cLogName
ENDIF

RETURN ""

STATIC FUNCTION GetLastEntry( cLog, /* @ */ nStart, /* @ */ nEnd )

LOCAL cLogHeaderExp := "\n[1-2][0-9][0-9][0-9]-[0-1][0-9]-[0-3][0-9] [0-2][0-9]:[0-6][0-9] [\S ]*"
Expand Down Expand Up @@ -277,63 +185,15 @@ STATIC FUNCTION GetLastEntry( cLog, /* @ */ nStart, /* @ */ nEnd )

RETURN cLog

STATIC FUNCTION MakeEntry( aChanges, cMyName, cLogName, lAllowChangeLog, cEOL )
STATIC FUNCTION MakeEntry( aChanges )

LOCAL nOffset := hb_UTCOffset()
LOCAL tDate := hb_DateTime()

LOCAL cLog
LOCAL cLog := "module: edit my changes" + hb_eol() + hb_eol()
LOCAL cLine

tDate -= ( nOffset / 86400 )
nOffset := 0

IF nOffset == 0
cLog := hb_StrFormat( "%1$s UTC", ;
hb_TToC( tDate, "yyyy-mm-dd", "hh:mm" ) )
ELSE
cLog := hb_StrFormat( "%1$s UTC+%2$s%3$02d%4$02d", ;
hb_TToC( tDate, "yyyy-mm-dd", "hh:mm" ), ;
iif( nOffset < 0, "-", "+" ), ;
Int( Abs( nOffset ) / 3600 ), ;
Int( Abs( nOffset ) % 3600 / 60 ) )
ENDIF

cLog += " " + cMyName + cEOL

FOR EACH cLine IN aChanges
IF lAllowChangeLog .OR. ! SubStr( cLine, 5 ) == hb_FNameNameExt( cLogName )
cLog += cLine + cEOL
ENDIF
NEXT

RETURN cLog

STATIC FUNCTION IsLastEntryEmpty( cLog, cLogName, /* @ */ lChangeLog )

LOCAL cLine

lChangeLog := .F.

FOR EACH cLine IN hb_ATokens( cLog, .T. )
IF ! cLine:__enumIsFirst()
IF Empty( Left( cLine, 2 ) ) .AND. ! Empty( SubStr( cLine, 3, 1 ) )
IF SubStr( cLine, 5 ) == hb_FNameNameExt( cLogName )
lChangeLog := .T.
ENDIF
ELSEIF ! Empty( cLine )
RETURN .F.
ENDIF
ENDIF
cLog += cLine + hb_eol()
NEXT

RETURN .T.

/* If it's a single mod, include only the change text,
otherwise include the whole entry.
NOTE: It was confusing to mix the two styles, so for
now always include the complete log message. */
STATIC FUNCTION EntryToCommitMsg( cLog )
RETURN cLog

STATIC FUNCTION VCSDetect( /* @ */ cVCSDir, /* @ */ cLocalRoot )
Expand Down Expand Up @@ -377,9 +237,6 @@ STATIC FUNCTION GitLocalRoot()

RETURN iif( nResult == 0, hb_DirSepAdd( hb_DirSepToOS( hb_StrReplace( cStdOut, Chr( 13 ) + Chr( 10 ) ) ) ), "" )

STATIC FUNCTION GitIsMerge( cGitDir )
RETURN hb_vfExists( cGitDir + "MERGE_MSG" )

STATIC FUNCTION GitFileList()

LOCAL cStdOut
Expand All @@ -397,14 +254,6 @@ STATIC FUNCTION GitFileList()

RETURN aList

STATIC FUNCTION GitUser()

LOCAL cName

hb_processRun( Shell() + " " + CmdEscape( "git config user.name" ),, @cName )

RETURN AllTrim( hb_StrReplace( cName, Chr( 10 ) + Chr( 13 ) ) )

STATIC FUNCTION GitEditor()

LOCAL cValue
Expand Down

0 comments on commit 0161a37

Please sign in to comment.