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

Line under module doc comment #3772

Merged
merged 2 commits into from
Jun 6, 2024
Merged

Line under module doc comment #3772

merged 2 commits into from
Jun 6, 2024

Conversation

B-rando1
Copy link
Collaborator

@B-rando1 B-rando1 commented Jun 4, 2024

We had discussed that there should be a blank line beneath the module-level doc comment at the top of files. It also fixes an issue I was having with the generated Julia code, which comes in handy.

This changes stable (175 files in stable, to be precise 😅). I didn't check every file, but I checked about half of them and they all have exactly one blank space under the module doc comment now.

The one thing I just wanted to double-check was, are there any cases where that extra line would break Doxygen (or whatever documentation generator the target uses)? I ran make doxygen and it ran fine, but I'm not too familiar with Doxygen so I thought it would be smart to ask. @bmaclach if you get a chance would you be able to weigh-in on DocC? Thanks.

Closes #3732

@samm82
Copy link
Collaborator

samm82 commented Jun 4, 2024

A one-line change that changes the generated code in 175 places? Very nice 😁 much better than doing this manually!

For tracking the benefits of Drasil, I wonder if we could create a label for instances in which the benefits of generative programming are obvious (so @smiths would have a nice place to look for examples, as well as future paper authors). Something like "benefits of Drasil" or "Drasil showcase" or just "showcase"?

@smiths
Copy link
Collaborator

smiths commented Jun 4, 2024

Yes, I agree with @samm82. We'll be looking for examples like this in the future and it would be nice to be able to find them. There aren't any labels for pull requests, so we can't just label the PR. We could label the issue, but the connection between an issue and a PR isn't always one to one. My suggestion is that we create a wiki page where we list our examples. The list can start with the current example, but there will be more. 😄 With the wiki we can have a link directly to the PR. We can also have an explanation of why it is a good example. @BilalM04 can you please create a wiki page? With the changes that @balacij has made you shouldn't edit the wiki directly. You will need to edit the files in the wiki folder in a branch and do a pull request.

@samm82
Copy link
Collaborator

samm82 commented Jun 4, 2024

@smiths We can label PRs (I just labelled this one enhancement)! Although I think a wiki with more details would also be a good idea

@smiths
Copy link
Collaborator

smiths commented Jun 4, 2024

Thank you @samm82. I didn't look hard enough to find the label field. 😄 We should definitely do that, and enhancement is a good label. I think the wiki is still a good idea to record the explanation of why the enhancement is so efficient, since that isn't always going to be highlighted in the PR notes.

@bmaclach
Copy link
Collaborator

bmaclach commented Jun 5, 2024

@B-rando1 I don't have any insight about whether the blank line would break DocC. I never did use DocC outside of the context of GOOL, so I don't actually know much more about it than you do, at this point.

@B-rando1
Copy link
Collaborator Author

B-rando1 commented Jun 5, 2024

No worries at all @bmaclach, thanks for getting back to us 🙂!

I'll take a quick look around tomorrow, and if everything looks good we should be able to merge after that.

@BilalM04
Copy link
Collaborator

BilalM04 commented Jun 6, 2024

@smiths which section in the sidebar should I put the wiki page for "Power of Drasil" under?

@B-rando1
Copy link
Collaborator Author

B-rando1 commented Jun 6, 2024

I think we should be ready to go ahead with the merge. I generated the Doxygen files again, and I was able to find the information from the header at the top, which means that Doxygen was able to find it. With Swift I can't be as confident, as none of the information online gives examples of code. I asked Copilot about it though, and it seems pretty confident that a blank line is ok.

@smiths do you think we're good to merge?

@smiths
Copy link
Collaborator

smiths commented Jun 6, 2024

Yes, I agree with @samm82. We'll be looking for examples like this in the future and it would be nice to be able to find them. There aren't any labels for pull requests, so we can't just label the PR. We could label the issue, but the connection between an issue and a PR isn't always one to one. My suggestion is that we create a wiki page where we list our examples. The list can start with the current example, but there will be more. 😄 With the wiki we can have a link directly to the PR. We can also have an explanation of why it is a good example. @BilalM04 can you please create a wiki page? With the changes that @balacij has made you shouldn't edit the wiki directly. You will need to edit the files in the wiki folder in a branch and do a pull request.

I inadvertently asked @BilalM04 to create the wiki entry when @B-rando1 has been the one working on this issue. The autocomplete (and my lack of proof reading) is to blame for this mistake. Maybe @BilalM04 and @B-rando1 can work together on this? Maybe @BilalM04 could create the page in the wiki and @B-rando1 could add the content describing this specific example. The main thing is to start this important piece of infrastructure so that when we are writing we can easily find great examples. 😄

@smiths
Copy link
Collaborator

smiths commented Jun 6, 2024

sidebar

With respect to where to put this in the sidebar, we can put it under Misc. Eventually we will need to redesign the wiki. Thank you @BilalM04.

@smiths
Copy link
Collaborator

smiths commented Jun 6, 2024

Yes, we are good to merge. @BilalM04 and @B-rando1 if we are at risk of not updating the wiki, please create an issue to remind us. 😄

@smiths smiths merged commit ef35006 into master Jun 6, 2024
5 checks passed
@smiths smiths deleted the LineUnderModDoc branch June 6, 2024 14:29
@B-rando1
Copy link
Collaborator Author

B-rando1 commented Jun 6, 2024

Sounds good. I'll create an issue so that we make sure to remember.

@BilalM04
Copy link
Collaborator

BilalM04 commented Jun 6, 2024

Sounds good! I've made the additions in this branch and was just about to create a PR. I'll link the new issue to the PR when @B-rando1 makes it.

@B-rando1
Copy link
Collaborator Author

B-rando1 commented Jun 6, 2024

I created the issue! @BilalM04 it's #3778.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generated Python header comment missing empty line beneath
6 participants