Skip to content

2.9 fix docs: entry should read and link to 'halcompile' not 'comp'#3448

Merged
andypugh merged 2 commits intoLinuxCNC:masterfrom
Sigma1912:2.9_fix_doc_link_halcompile
Jun 15, 2025
Merged

2.9 fix docs: entry should read and link to 'halcompile' not 'comp'#3448
andypugh merged 2 commits intoLinuxCNC:masterfrom
Sigma1912:2.9_fix_doc_link_halcompile

Conversation

@Sigma1912
Copy link
Copy Markdown
Contributor

No description provided.

@Sigma1912 Sigma1912 force-pushed the 2.9_fix_doc_link_halcompile branch from e111c58 to 6459c7a Compare May 23, 2025 08:21
@pcw-mesa
Copy link
Copy Markdown
Collaborator

The "halcomplle" change makes sense in the glossary but its wrong on the hal components list
on the hal components list, "comp" is correct but its description is wrong.

@Sigma1912
Copy link
Copy Markdown
Contributor Author

'comp' is listed twice in the components list, it's in section 1.6.2 and in section 1.10:
https://linuxcnc.org/docs/html/hal/components.html

@pcw-mesa
Copy link
Copy Markdown
Collaborator

pcw-mesa commented May 23, 2025

Right, but in any case, halcompile does not belong in a a real time components list.

Ok I see, I was looking at the document referenced in the forum which has a different issue

@Sigma1912
Copy link
Copy Markdown
Contributor Author

I agree. So should this be anywhere on that page or just be removed?

@andypugh andypugh merged commit 775a6f3 into LinuxCNC:master Jun 15, 2025
5 of 8 checks passed
@Sigma1912 Sigma1912 deleted the 2.9_fix_doc_link_halcompile branch June 16, 2025 05:32
@BsAtHome
Copy link
Copy Markdown
Contributor

My review comments were ignored. And, this broke CI.

@andypugh
Copy link
Copy Markdown
Collaborator

My review comments were ignored. And, this broke CI.

I am puzzled why it broke CI. It was a tiny change in docs.

But, where were your review comments? I didn't see anything in #3468 relevant to this small update.

@BsAtHome
Copy link
Copy Markdown
Contributor

I am puzzled why it broke CI. It was a tiny change in docs.

So am I... However, this PR didn't pass CI's htmldocs and pkg-indep.

asciidoc: ERROR: components.adoc: line 409: [tabledef-default] total width less than 100%: 0
asciidoc: ERROR: components.adoc: line 409: [tabledef-default] missing leading separator: (?ms)((?<!\S)((?P<span>[\d.]+)(?P<op>[*+]))?(?P<align>[<\^>.]{,3})?(?P<style>[a-z])?)?\|
asciidoc: WARNING: components.adoc: line 414: table row 1: empty spanned row
...

So I guess that the problem is a formatting one.

But, where were your review comments? I didn't see anything in #3468 relevant to this small update.

You are referencing my issue. The review comments are in this PR. You should be able to see them.

@Sigma1912
Copy link
Copy Markdown
Contributor Author

You should be able to see them.

I see nothing. Also I would have expected that a review comment would trigger an email message but I'm not positive about that.

@BsAtHome
Copy link
Copy Markdown
Contributor

Here is a screenshot of my window... darn github refuses my PNG.

Here is a link to the screenshot.

@Sigma1912
Copy link
Copy Markdown
Contributor Author

Here is what I see:

screenshot

@andypugh
Copy link
Copy Markdown
Collaborator

Anyway, I agree that halcompile doesnt belong in realtime components, and removed it. The glossary is correct already.
So, I think that both your comments were addressed?

@BsAtHome
Copy link
Copy Markdown
Contributor

Except that 775a6f3#diff-7410ca79900a698580908e783c156c50a2747f8fb65c8e221d2c70f4d0da832f removes the |=== tag to start a table. That is plain wrong.

@Sigma1912
Copy link
Copy Markdown
Contributor Author

Yes that's obviously a mistake, apologies for that. Thanks

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.

4 participants