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

doc: provide include path list for doxyfile #4890

Merged
merged 1 commit into from
Mar 23, 2016
Merged

doc: provide include path list for doxyfile #4890

merged 1 commit into from
Mar 23, 2016

Conversation

A-Paul
Copy link
Member

@A-Paul A-Paul commented Feb 24, 2016

Intended as a solution to #4794.
The effective goal is: Provide the doxygen tag STRIP_FROM_INC_PATH with a list of all actual riot include paths.
This is a first approach. Though it works as intended, improvements are welcome.

@A-Paul A-Paul added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Area: doc Area: Documentation Area: build system Area: Build system labels Feb 24, 2016
@A-Paul A-Paul mentioned this pull request Feb 24, 2016
Closed
LudwigKnuepfer
LudwigKnuepfer reviewed Feb 24, 2016
View reviewed changes
Makefile
@@ -7,15 +7,19 @@ all: welcome
@exit 1

doc:
export STRIP_FROM_INC_PATH_LIST=$(find . -type d -name include -printf " \"$PWD/%P\"")
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest looking for possibilities of obtaining the directory structure that is actually under version control using git. The scripts in dist/tools might serve as a source of inspiration.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, will look there. I was curious if RIOT's make system could have provide such a list instead.

Copy link
Member

Choose a reason for hiding this comment

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

Not that I'm aware of .. @Kijewski ?

Copy link
Member Author

Choose a reason for hiding this comment

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

After grepping through the makefiles, I assume that there is a focus at the subset of include dirs dependent to a specific configuration of board, cpu, modules...
Having all possible includes is not a build case.

@A-Paul
Copy link
Member Author

A-Paul commented Feb 24, 2016

@LudwigKnuepfer thanks for your suggestions. Come back with a better approach.

@OlegHahm
Copy link
Member

Any progress here?

@OlegHahm
Copy link
Member

Is this still WIP?

@OlegHahm
Copy link
Member

Hm, while http://doc.riot-os.org/unionipv6__addr__t.html would show the correct include with this PR, http://doc.riot-os.org/ipv6_8h.html still shows #include "ipv6/addr.h" instead of #include "net/ipv6/addr.h".

@A-Paul
Copy link
Member Author

A-Paul commented Mar 23, 2016

@OlegHahm, wow, you're fast. :-)
I think the answer to your question regarding the WIP state depends on what you are investigating now. The impact on the generated docs.

Looking at your two examples, the main difference is that the first is a include generated by doxygen, and the includes in the second are just taken from the header file.

@OlegHahm
Copy link
Member

Looking at your two examples, the main difference is that the first is a include generated by doxygen, and the includes in the second are just taken from the header file.

Ah, yes, you're right. Good observation. Okay, in this case we should fix this directly in the header.

@OlegHahm
Copy link
Member

Hm, looking at some sample outputs and looking at the diff of this PR, I would say that this PR is definitely an improvement and should be merged rather earlier than later. If you or someone else finds other improvements to this, we can handle this in a later PR. Do you agree?

@OlegHahm
Copy link
Member

I prepared a small PR for the other fix in #5159.

@A-Paul
Copy link
Member Author

A-Paul commented Mar 23, 2016

@OlegHahm, are you asking me to ACK my own PR? :-)
At least, I think it does what #4794 has specified.

@A-Paul
Copy link
Member Author

A-Paul commented Mar 23, 2016

I remove WIP set milestone and squash, ok?

@OlegHahm
Copy link
Member

I'm asking if you agree that this PR is ready to be merged and the WIP label can be removed.

@OlegHahm
Copy link
Member

I remove WIP set milestone and squash, ok?

Okay - and I say ACK! :)

@A-Paul A-Paul removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Mar 23, 2016
@A-Paul A-Paul added this to the Release 2016.04 milestone Mar 23, 2016
@A-Paul A-Paul added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 23, 2016
@OlegHahm OlegHahm merged commit d702793 into RIOT-OS:master Mar 23, 2016
@A-Paul A-Paul deleted the doxygen_relpath branch January 17, 2017 10:55
@A-Paul A-Paul mentioned this pull request Jan 17, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@LudwigKnuepfer LudwigKnuepfer

Labels
Area: build system Area: Build system Area: doc Area: Documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
Release 2016.04
Development

Successfully merging this pull request may close these issues.
3 participants
@A-Paul @OlegHahm @LudwigKnuepfer