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

Week2 Blogpost #800

Merged
merged 9 commits into from
Jun 19, 2023
Merged

Week2 Blogpost #800

merged 9 commits into from
Jun 19, 2023

Conversation

JoaoDell
Copy link
Contributor

Here is my week 2 blogpost everyone, let me know of any changes needed

Copy link
Contributor

@ganimtron-10 ganimtron-10 left a comment

Choose a reason for hiding this comment

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

Hey @JoaoDell ,
The blogpost was sucessfully generated locally but as you can see below the code section is not properly rendered as code. Secondly the image is also not loaded.
Please look into these issues otherwise it lgtm.

image

@@ -0,0 +1,80 @@
Week 2: The Importance of (good) Documentation
=====================
Copy link
Contributor

Choose a reason for hiding this comment

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

The = symbol should cover the whole title.

@JoaoDell
Copy link
Contributor Author

Thanks @ganimtron-10, do you know how can I properly render it as code? This has been a struggle. I will see what is up with the image as it is loading when I build it here, does github have anything related to privacy on images or something? I checked and my repo is public

@JoaoDell
Copy link
Contributor Author

okay actually I forgot about the "::" usage will change that now 😵‍💫

@JoaoDell
Copy link
Contributor Author

Just updated with the code properly rendered. However, I am still trying to figure out the problem with the image. What I done was to upload it to a personal repo named gsoc_assets and get the image link by "Open image in another page". It seems my link is different of yours previous uploaded images as they are on user-images, how do you proceed with using images in rst @ganimtron-10?

Copy link
Contributor

@tvcastillod tvcastillod left a comment

Choose a reason for hiding this comment

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

@JoaoDell Nice blogpost. Looks good overall, just check one comment below.




.. image:: https://github.com/JoaoDell/gsoc_assets/main/images/allocate-2d-3d.png
Copy link
Contributor

Choose a reason for hiding this comment

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

I can see the image well

image

Copy link
Contributor

Choose a reason for hiding this comment

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

same, it works!

}

This slight difference is significant: while in Allocate2D the program immediately fails, in Allocate3D the function is simply returned
**false**, with its error pushed to vtkErrorMacro. I could have realised that earlier if I were using vtkErrorMacro, but this contrastant
Copy link
Contributor

Choose a reason for hiding this comment

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

contrastant sounds weird to me, maybe you can use another word.

@ganimtron-10
Copy link
Contributor

It seems my link is different of yours previous uploaded images as they are on user-images, how do you proceed with using images in rst @ganimtron-10?

The approach you are using for adding image is more better and will work as a permanent solution, whereas the user-images are not under our control and may get deleted.
Still if you want to know how I did it last time, you may have noticed that I had a PR #790 where I had added the images in the description I got link of those images and added them into my post.
Otherwise when ever you paste an image in any issue body or PR body you get a user-image link that too works.

Copy link
Contributor

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

Nice blog post @JoaoDell !

See below my comments

-----------------------

Last week, I was facing some issues with a VTK feature essential so I could move forward with my project: Framebuffer Objects.
As described in my `last blogpost <https://blogs.python-gsoc.org/en/joaodellaglis-blog/the-fbo-saga-week-1/>`_, for some reason the 2D allocation methods for it weren't working.
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be better to do a local reference. In rst, you can do a reference to another file and link will appears during the build.

Internal link have a higher priority than external link.

Comment on lines 74 to 76
This slight difference is significant: while in Allocate2D the program immediately fails, in Allocate3D the function is simply returned
**false**, with its error pushed to vtkErrorMacro. I could have realised that earlier if I were using vtkErrorMacro, but this difference in their
implementation made it harder for me and my mentors to realise what was happening.
Copy link
Contributor

Choose a reason for hiding this comment

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

nice catch! interesting

return false;
}

This slight difference is significant: while in Allocate2D the program immediately fails, in Allocate3D the function is simply returned
Copy link
Contributor

Choose a reason for hiding this comment

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

Allocate2D
Allocate3D

mark them everywhere like you did for assert(this->Context);




While in Allocate2D there is an ``assert(this->Context);``, in Allocate3D the assertion is translated into:
Copy link
Contributor

Choose a reason for hiding this comment

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

Allocate2D
Allocate3D

mark them everywhere like you did for assert(this->Context);




.. image:: https://github.com/JoaoDell/gsoc_assets/main/images/allocate-2d-3d.png
Copy link
Contributor

Choose a reason for hiding this comment

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

same, it works!

color_texture.SetMinificationFilter(0)
color_texture.SetMagnificationFilter(0)

The code worked fine. But as my last blogpost showed, Allocate3D() method worked just fine without a (visible) problem, why is that?
Copy link
Contributor

Choose a reason for hiding this comment

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

Allocate2D
Allocate3D

mark them everywhere like you did for assert(this->Context);

@JoaoDell
Copy link
Contributor Author

Hey @skoudoro I just made the changes you recommended. Is the way I changed the link the way you desired?

@skoudoro
Copy link
Contributor

This might answer your question concerning link : https://stackoverflow.com/questions/37553750/how-can-i-link-reference-another-rest-file-in-the-documentation

currently, your change is incorrect concerning the link

@codecov
Copy link

codecov bot commented Jun 16, 2023

Codecov Report

Merging #800 (a053fe0) into master (4160b1d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #800   +/-   ##
=======================================
  Coverage   84.32%   84.32%           
=======================================
  Files          43       43           
  Lines       10173    10173           
  Branches     1383     1383           
=======================================
  Hits         8578     8578           
  Misses       1239     1239           
  Partials      356      356           

@JoaoDell
Copy link
Contributor Author

I guess it may work now @skoudoro, i have built it locally with the instructions the documentation had about linking and it worked, see if it works for you. Thanks for the tip 👍

Copy link
Contributor

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

Looks great! thanks for the update, merging

@skoudoro skoudoro merged commit 224c52a into fury-gl:master Jun 19, 2023
22 of 30 checks passed
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.

4 participants