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.

Sign up for GitHub

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

Jump to bottom

Update top-level haddocks for consensus #4197

Merged
merged 3 commits into from
Dec 14, 2022
Merged

Update top-level haddocks for consensus #4197

merged 3 commits into from
Dec 14, 2022

Conversation

abailly-iohk
Copy link
Contributor

@abailly-iohk abailly-iohk commented Nov 29, 2022 •
edited
Loading

Description

Fix #4149 #4057

This PR introduces a section about Consensus Components in the prolog of generated haddock documentation, and add a couple diagrams to try to provide a better overview of what's going on in the consensus codebase.

Screenshot 2022-11-29 at 12 30 37

Checklist

  • Branch
    • Commit sequence broadly makes sense
    • Commits have useful messages
    • New tests are added if needed and existing tests are updated
    • If this branch changes Consensus and has any consequences for downstream repositories or end users, said changes must be documented in interface-CHANGELOG.md
    • If this branch changes Network and has any consequences for downstream repositories or end users, said changes must be documented in interface-CHANGELOG.md
    • If serialization changes, user-facing consequences (e.g. replay from genesis) are confirmed to be intentional.
  • Pull Request
    • Self-reviewed the diff
    • Useful pull request description at least containing the following information:
      • What does this PR change?
      • Why these changes were needed?
      • How does this affect downstream repositories and/or end-users?
      • Which ticket does this PR close (if any)? If it does, is it linked?
    • Reviewer requested

coot
coot requested changes Nov 29, 2022
View reviewed changes
scripts/prolog Outdated Show resolved Hide resolved
scripts/prolog Outdated Show resolved Hide resolved
scripts/prolog Outdated Show resolved Hide resolved
scripts/prolog Outdated Show resolved Hide resolved
@abailly-iohk
Copy link
Contributor Author

@coot I removed previous existing sections referring to io-sim and win32-network. Should have been another PR or a dedicated commit but 🤷

@abailly-iohk
Copy link
Contributor Author

I have addressed the requested changes and I am reluctant to start working on something else before this PR gets merged. @coot @dnadales WDYT?

dnadales
dnadales reviewed Dec 6, 2022
View reviewed changes
scripts/haddocs.sh
@@ -92,3 +100,8 @@ for file in $(ls $OUTPUT_DIR/*/doc-index.json); do
> /tmp/doc-index.json
mv /tmp/doc-index.json "${OUTPUT_DIR}/doc-index.json"
done

# Copy modules map to output directory
# TODO: dynamically generate
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to create an issue and address it rather soon?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely.

dnadales
dnadales approved these changes Dec 6, 2022
View reviewed changes
Copy link
Member

@dnadales dnadales left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you.

@abailly-iohk abailly-iohk mentioned this pull request Dec 6, 2022
Open
@abailly-iohk
Copy link
Contributor Author

@coot @dnadales It seems this PR is blocked and has failing CI tasks which is annoying. I had a look at those failures and they seem completely unrelated to my changes, but having a red CI is definitely a blocker. I will try to rebase on master to see if that changes anything,

@abailly-iohk abailly-iohk force-pushed the abailly-iohk/update-haddocks-for-consensus branch from 1bb6d24 to 9259d6f Compare December 13, 2022 13:13
coot
coot approved these changes Dec 13, 2022
View reviewed changes
Copy link
Contributor

@coot coot left a comment

Choose a reason for hiding this comment

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

LGTM

@dnadales
Copy link
Member

bors r+

iohk-bors bot added a commit that referenced this pull request Dec 14, 2022
@iohk-bors @abailly-iohk
Merge #4197
7f1eff5 
4197: Update top-level haddocks for consensus r=dnadales a=abailly-iohk

# Description

Fix #4149 #4057

This PR introduces a section about `Consensus Components` in the prolog of generated haddock documentation, and add a couple diagrams to try to provide a better overview of what's going on in the consensus codebase.

<img width="1094" alt="Screenshot 2022-11-29 at 12 30 37" src="https://user-images.githubusercontent.com/79840582/204518117-a6fdc182-5393-42d8-8022-3c64b4aebfa2.png">



Co-authored-by: Arnaud Bailly <arnaud.bailly@iohk.io>
Co-authored-by: Arnaud Bailly <79840582+abailly-iohk@users.noreply.github.com>
@dnadales dnadales merged commit 0fe3291 into master Dec 14, 2022
@iohk-bors iohk-bors bot deleted the abailly-iohk/update-haddocks-for-consensus branch December 14, 2022 09:58
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Dec 14, 2022

Timed out.

@abailly abailly mentioned this pull request Dec 16, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dnadales dnadales dnadales approved these changes

@coot coot coot approved these changes

@bolt12 bolt12 Awaiting requested review from bolt12

@nfrisby nfrisby Awaiting requested review from nfrisby

@bartfrenk bartfrenk Awaiting requested review from bartfrenk

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Provide an hyperlinked map of consensus modules
3 participants
@abailly-iohk @dnadales @coot