Skip to content

Conversation

@mmattar
Copy link

@mmattar mmattar commented Mar 14, 2018

  • Limited to C# classes that are user-facing
  • Added a placeholder for Python to signify intentions
  • Generated doxygen pages

- Limited to C# classes that are user-facing/
- Added a placeholder for Python to signify intentions
- Generated doxygen pages
@vincentpierre
Copy link
Contributor

Are we supposed to have the doxygen documents in the repo ? Shouldn't this be hosted somewhere ?

@vladimiroster
Copy link

Keeping generated files in the repo is usually bad practice.

@jo3w4rd
Copy link
Collaborator

jo3w4rd commented Mar 14, 2018

In this case, it lets us host the docs, including the API reference, from Github (using github pages).

@mmattar
Copy link
Author

mmattar commented Mar 14, 2018

@jo3w4rd - we should do that for the next (minor) release. For now, I've updated the pages to provide documentation on how to generate them yourself.

@vincentpierre + @vladimiroster : please review new version.

@xiaomaogy
Copy link
Contributor

Do we have a ml-agents.conf file anywhere? I don't see it in the repo, and not in this pull request, but it is noted in the API-Reference.md.

@jo3w4rd
Copy link
Collaborator

jo3w4rd commented Mar 14, 2018

The file is named "dox-ml-agents.conf". Were you planning to rename it @mmattar?

@mmattar
Copy link
Author

mmattar commented Mar 14, 2018

Great catch folks - the current name works for me. Just pushed a change.

# Conflicts:
#	docs/doxygen/Readme.md
Copy link
Collaborator

@jo3w4rd jo3w4rd left a comment

Choose a reason for hiding this comment

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

We will need to make it more clear which parts of the API a developer needs to use and which parts are internal. But that's a bigger project then we should bite off right now.


doxygen dox-ml-agents.conf

`ml-agents.conf` is a configuration file that includes the classes
Copy link
Contributor

Choose a reason for hiding this comment

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

ml-agents.conf ---> dox-ml-agents.conf

Copy link
Author

Choose a reason for hiding this comment

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

Great catch - fixed!

Feature-Monitor.md \
Limitations-and-Common-Issues.md \
../unity-environment/Assets/ML-Agents/Scripts \
../python/unityagents
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we only include a portion of the files in the ../unity-environment/Assets/ML-Agents/Scripts and ignoring files like Communicator.cs and BCTeacherHelper.cs?

Copy link
Author

Choose a reason for hiding this comment

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

We've only included the files that the developers will be using or extending. We do need to add ALL in the future.

@mmattar mmattar merged commit 0e3b671 into development-0.3 Mar 14, 2018
@mmattar mmattar deleted the docs/doxygen branch March 14, 2018 19:13
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants