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

Documentation improvements #3526

Merged
merged 2 commits into from
Jul 29, 2024
Merged

Documentation improvements #3526

merged 2 commits into from
Jul 29, 2024

Conversation

roman-khimov
Copy link
Member

Fix some of the old bugs related to this.

@@ -33,6 +33,13 @@ a dialect of Go rather than a complete port of the language:
it's up to the programmer whether assert can be performed successfully.
Copy link
Contributor

Choose a reason for hiding this comment

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

Format of the commit message is questionable. @AnnaShaleva would not like that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

yes,

The following format is expected:

subsystem: short description (up to 80 symbols, better up to 60)

Long description with references, links...

in the first line there are no links or refs. At least they are not mentioned in https://github.com/nspcc-dev/.github/blob/master/git.md#commit-messages

Copy link
Member Author

Choose a reason for hiding this comment

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

While I think it's compliant (there are no limits of what short description can or can not include as long as it's short) and there are numerous commits like f4b61b5, b1bb12d, d2a7162, eade327 and others, I can at the same time say that @AnnaShaleva has never wrote a short description like that in ~1715 commits that I can see in this repo (~116 commits in neo-bench are the same). I think it's still OK, but maybe we can have our policies improved one way or another.

Copy link
Member

@AnnaShaleva AnnaShaleva Aug 5, 2024

Choose a reason for hiding this comment

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

in the first line there are no links or refs.

I'm OK with the commit formatting presented in this PR, links are allowed both in the commit header and in the body. It's just about my personal preferences, I try to keep my own commits with links in the body, not in the header.

@@ -403,3 +403,25 @@ where:

Note that `Transaction` is a NeoGo extension that isn't supported by the NeoC#
node and must be disabled on the public Neo N3 networks.

## DB compatibility
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs the same indent as above.

Copy link
Member Author

Choose a reason for hiding this comment

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

We have a list above and this is a new section, but I've improved the list a bit anyway.

@roman-khimov roman-khimov merged commit 07da75c into master Jul 29, 2024
1 check passed
@roman-khimov roman-khimov deleted the improve-doc branch July 29, 2024 12:07
Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

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

LGTM except a tiny #3545.

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.

3 participants