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

Reorganize folders of DataScienceTutorials.jl #218

Merged
merged 10 commits into from
Apr 17, 2024
Merged

Reorganize folders of DataScienceTutorials.jl #218

merged 10 commits into from
Apr 17, 2024

Conversation

EssamWisam
Copy link
Collaborator

This includes removing useless files, reorganizing common ones together, centralizing the routing index at the top-level and most importantly using a folder layout instead of prefixes to distinguish different tutorial types.

@EssamWisam EssamWisam requested a review from ablaom April 15, 2024 18:18

* Be sure the version of Julia declared near the top of `index.md`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This refers to the old index.md. The referred to part is now in how-to-run-code hence the change.

@EssamWisam
Copy link
Collaborator Author

image

Why does this heading exist? Old Instructions gives the impression that there are "alternative instructions"; meanwhile, in reality they do add information about exactly three things :

  • How to visualize after changing (e.g., adding a new tutorial)
  • Adding plots
  • Troubleshooting

I would vote for the following:

  • Get rid of the potentially misleading "Old Instructions" title
  • So far, I added briefly how to visualize the changes as the last step in the "add tutorial section" instead I can link to here (which is less brief and more informative) or get rid of this section.
  • The "Adding Plots" section is not absolutely necessary because it was mentioned earlier to follow the blueprint by copying another tutorial which makes that easy. Thus, we can either remove this section or keep it and link to it in the "adding tutorials section. In the latter case, we have to remove what's mentioned there on PyPlot
  • Troubleshooting section should stay.

@ablaom
Copy link
Member

ablaom commented Apr 15, 2024

@EssamWisam I'm very happy for you to make an executive decision about these details. Feel free to rewrite these instructions as much as you like.

Since the original developer has moved on, just go ahead do your best. You are the active maintainer with the most knowledge about the how the process now works and I will defer to you on this.

Do ping me when you are ready for final review.

@EssamWisam
Copy link
Collaborator Author

Alright. Ready for review. Here is a pretty print of the new README for easier review. The rest as can be seen are folder organization changes. I tested rendering the website after I did those.

README.md Outdated Show resolved Hide resolved
Co-authored-by: Anthony Blaom, PhD <[email protected]>
README.md Outdated Show resolved Hide resolved

### Fixing an existing tutorial
* Find the corresponding Julia script in `_literate` and fix it in a PR.
* Ensure it works and renders properly as explained in [this section](#👀-visualise-modifications-locally).
Copy link
Member

Choose a reason for hiding this comment

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

This internal link (and another like it) do not seem to work, at least in the preview.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, I followed this which claims it works on Github and it does work locally for me. Let's merge and if it doesn't work I will make rightaway make a PR when I see it to replace it with "as in the... section below" or similar.

Copy link
Member

@ablaom ablaom left a comment

Choose a reason for hiding this comment

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

This is looks great, thanks. Just the few minor points.

Co-authored-by: Anthony Blaom, PhD <[email protected]>
@EssamWisam EssamWisam merged commit ca6a1a7 into master Apr 17, 2024
1 check passed
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.

2 participants