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

Concise info on creating and using a module without core and base #46628

Merged
merged 6 commits into from
Sep 12, 2022
Merged

Concise info on creating and using a module without core and base #46628

merged 6 commits into from
Sep 12, 2022

Conversation

udohjeremiah
Copy link
Contributor

I had a hard time solving this - (using @eval to write code into the module and then writing a function doc for the new function evaluated using @eval).

Thank God for the helpful and timely answers I got from slack and discourse, then I finally knew how to use @eval on working with functions on the module without core and base.

Then finally documenting the function was another series of asking questions and finally the timely answers came. And I could add a function docstring to it.

But the point is, why not just show a quick demo of this in the documentation so others don't have to pass through this stress?

@udohjeremiah udohjeremiah changed the title Concise info on creating a module without core and base Concise info on creating and using a module without core and base Sep 4, 2022
@ViralBShah ViralBShah added the docs This change adds or pertains to documentation label Sep 5, 2022
doc/src/manual/modules.md Outdated Show resolved Hide resolved
doc/src/manual/modules.md Outdated Show resolved Hide resolved
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Sep 6, 2022
@KristofferC
Copy link
Member

Whitespace check found 1 issues:
--
  | doc/src/manual/modules.md:338 -- trailing whitespace
  | make: *** [Makefile:106: check-whitespace] Error 1

@udohjeremiah
Copy link
Contributor Author

Whitespace check found 1 issues:

  | doc/src/manual/modules.md:338 -- trailing whitespace
  | make: *** [Makefile:106: check-whitespace] Error 1

I think its fixed.

@vtjnash vtjnash removed the merge me PR is reviewed. Merge when all tests are passing label Sep 7, 2022
@vtjnash
Copy link
Member

vtjnash commented Sep 7, 2022

Maybe it shouldn't try to test adding docstrings from the REPL (or add a more trivial one)?

@udohjeremiah
Copy link
Contributor Author

Maybe it shouldn't try to test adding docstrings from the REPL (or add a more trivial one)?

Yeah I've added a more trivial one which shouldn't throw an error.

@udohjeremiah udohjeremiah added the merge me PR is reviewed. Merge when all tests are passing label Sep 7, 2022
@Keno
Copy link
Member

Keno commented Sep 7, 2022

I still don't think the @doc and help?> examples here make any sense. They're a complete side track to anything being discussed here. It's really not a good idea to just throw in random unrelated concepts in examples.

@udohjeremiah
Copy link
Contributor Author

udohjeremiah commented Sep 7, 2022

I still don't think the @doc and help?> examples here make any sense. They're a complete side track to anything being discussed here. It's really not a good idea to just throw in random unrelated concepts in examples.

And I think they're one of the reasons the whitespace is failing as well.

I'll remove it now.

@ViralBShah ViralBShah removed the merge me PR is reviewed. Merge when all tests are passing label Sep 11, 2022
@udohjeremiah
Copy link
Contributor Author

@ViralBShah are you looking at this since you're more active in doc PRs.

@ViralBShah
Copy link
Member

I usually merge the ones that are obvious to me.

@udohjeremiah
Copy link
Contributor Author

@ViralBShah I certainly do not know what you mean by "obvious" but I feel this discussion 👉#46536 opened here concerning merging stuffs like this is a valid one. You might to check out what I'm talking about.

@Keno Keno merged commit 7eacf1b into JuliaLang:master Sep 12, 2022
@udohjeremiah udohjeremiah deleted the patch-10 branch September 12, 2022 19:30
@ViralBShah
Copy link
Member

Doc PRs touch many different aspects and need to have reviews from appropriate people. Thus, I can only merge the ones that I am sure about - on others I try to find other reviewers. But yes, developer bandwidth continues to be limited, although we try to do our best to stay on top of open PRs. A lot of people have commit access to the repo, but on a given PR there's often only a few people who can authoritatively merge.

I am happy to be pinged on such PRs to take a look.

@udohjeremiah
Copy link
Contributor Author

OK!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants