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

feat:refer viewpoints from tables #532

Merged
merged 8 commits into from
Oct 27, 2023

Conversation

majimaccho
Copy link
Contributor

@majimaccho majimaccho commented Oct 26, 2023

fix: #526
based on discussion in #530.

To render link to viewpoints from the markdown, the OutputTable function has to know the index of the viewpoints, which was not included in Viewpoint type.

So I added TableViewpoint type like below.

struct TableViewpoint {
    Index Int
    Name String
    Desc String
}

@majimaccho majimaccho marked this pull request as ready for review October 26, 2023 00:37
@majimaccho majimaccho marked this pull request as draft October 26, 2023 00:37
@@ -129,6 +129,11 @@ func (m *Md) OutputViewpoint(wr io.Writer, i int, v *schema.Viewpoint) error {

// Output generate markdown files.
func Output(s *schema.Schema, c *config.Config, force bool) (e error) {
s, err := s.SetViewpointsToTables()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not quite sure if this is is appropriate to set viewpoints to table.
I could be better to be in config.Analyze but it can effect more files.

Copy link
Owner

Choose a reason for hiding this comment

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

Let's start with the minimum effect range.

I like refactoring 👍

@majimaccho
Copy link
Contributor Author

@k1LoW
https://github.com/k1LoW/tbls/actions/runs/6647754723/job/18063703113?pr=532
reviewdoc's golangci-lint timed out sometimes.
You may want extend timeout (e.g. 5 minutes).
If you want, I can include it in this PR.

@majimaccho majimaccho marked this pull request as ready for review October 26, 2023 00:42
@k1LoW
Copy link
Owner

k1LoW commented Oct 26, 2023

If you want, I can include it in this PR.

Thank you!!!! Please!

@majimaccho majimaccho force-pushed the feat/refer-viewpoints-from-tables branch from 65ad9b7 to 32e89fc Compare October 26, 2023 01:40
@majimaccho majimaccho force-pushed the feat/refer-viewpoints-from-tables branch from 32e89fc to adc687d Compare October 26, 2023 01:56
@k1LoW
Copy link
Owner

k1LoW commented Oct 26, 2023

@majimaccho
Copy link
Contributor Author

Lint is just passed now by adc687d
Now golden files was missing so I added them

@majimaccho
Copy link
Contributor Author

@k1LoW
Sorry still tests are failing, I'd convert this to draft.

@majimaccho majimaccho marked this pull request as draft October 26, 2023 02:24
@k1LoW
Copy link
Owner

k1LoW commented Oct 26, 2023

Feel free to trial and error without worrying about it 👍

@majimaccho majimaccho requested a review from k1LoW October 26, 2023 07:33
@majimaccho majimaccho marked this pull request as ready for review October 26, 2023 07:33
output/md/md.go Outdated
for _, v := range t.Viewpoints {
data := []string{
fmt.Sprintf("[%s](viewpoint-%d.md)", v.Name, v.Index),
v.Desc,
Copy link
Owner

Choose a reason for hiding this comment

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

I would like to change the contents of the display using showOnlyFirstParagraph

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@k1LoW
Alright! I fixed it 👍

@majimaccho majimaccho requested a review from k1LoW October 26, 2023 08:23
Copy link
Owner

@k1LoW k1LoW left a comment

Choose a reason for hiding this comment

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

GREAT! Thank you!

@k1LoW k1LoW added enhancement New feature or request minor breaking-change labels Oct 27, 2023
@k1LoW k1LoW merged commit 2a2e7a9 into k1LoW:main Oct 27, 2023
2 checks passed
@github-actions github-actions bot mentioned this pull request Oct 27, 2023
@majimaccho majimaccho deleted the feat/refer-viewpoints-from-tables branch October 27, 2023 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refer viewpoints from tables
2 participants