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

Fix intermediate state where newly gen frontmatter wasn't saved yet #558

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sourishkrout
Copy link
Member

Also simplifies logic when document-level lifecycle identity (i.e. doc ID) is turned off.

func (f *Frontmatter) Marshal(requireIdentity bool) ([]byte, error) {
// Marshal returns a marshaled frontmatter including triple-dashed lines and a document ID.
// If the identity is required, but Frontmatter is nil, a new one including ID is created.
func (f *Frontmatter) Marshal(requireIdentity bool) ([]byte, string, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remark: In the ideal world we would strive for following signature: Marshal() ([]byte, error).

Copy link
Member Author

Choose a reason for hiding this comment

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

I know, but it would require me to rethink/rewrite a large part of the prepopulating frontmatter scenario which makes other things super complicated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Still need to do more integration testing with the extension before committing to this change, fwiw.

Thanks for the review!

Copy link

sonarcloud bot commented Apr 18, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
55.6% Coverage on New Code (required ≥ 70%)

See analysis details on SonarCloud

@sourishkrout sourishkrout marked this pull request as draft May 3, 2024 18:19
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.

None yet

2 participants