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

fixed issue in webglColormapGenertor file #330

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Somya-Singhal
Copy link
Contributor

I tried to fix issues as I mentioned in issue #313 in webglColormapGenerator.js file

@gitpod-io
Copy link

gitpod-io bot commented Apr 6, 2022

@jywarren
Copy link
Member

jywarren commented Apr 7, 2022

Hi @Somya-Singhal do you know what effects this was having on the application before the fixes? Is there any way to show the output changing from before/after, so I can follow along in what effect these changes have? Thank you so much!!

@Somya-Singhal
Copy link
Contributor Author

Hi @Somya-Singhal do you know what effects this was having on the application before the fixes? Is there any way to show the output changing from before/after, so I can follow along in what effect these changes have? Thank you so much!!

Hello @jywarren , please correct me if I am wrong anywhere.
At first, I tried to look at what this program was doing and according to me, it is used for shading the appropriate levels of light, darkness and colour within the image then when I looked for the before and after changes, I found that no changes were made in the application. Please could you guide me as so that I could know what was the issue and move ahead to fix it.

@jywarren
Copy link
Member

Hi, of course -- so, you have the right idea on the functionality. I'm trying to review your contribution so I wanted to know:

  1. did the code do something I can see that is "wrong" before your changes
  2. what is the new behavior your code produces

That way I can try the old version, and start up your new version, and compare how each one works. If the issue you describe in 1) is resolved, i can approve this and we can merge it!

If the answer to 1) is that nothing was broken, but that the changes are good syntax and good formatting, but that the behavior of the program won't change, that is also OK. I can then just try your new version and be sure it still produces the same output when we use a color map, so nothing has been broken. Then i'll approve this. Does that make sense? Thanks!!

@Somya-Singhal
Copy link
Contributor Author

Hi, of course -- so, you have the right idea on the functionality. I'm trying to review your contribution so I wanted to know:

  1. did the code do something I can see that is "wrong" before your changes
  2. what is the new behavior your code produces

That way I can try the old version, and start up your new version, and compare how each one works. If the issue you describe in 1) is resolved, i can approve this and we can merge it!

If the answer to 1) is that nothing was broken, but that the changes are good syntax and good formatting, but that the behavior of the program won't change, that is also OK. I can then just try your new version and be sure it still produces the same output when we use a color map, so nothing has been broken. Then i'll approve this. Does that make sense? Thanks!!

Ok, I understood @jywarren, I will check it and be sure nothing is broken and then update you. Thanks for clarifying my doubt!

@jywarren
Copy link
Member

jywarren commented Apr 13, 2022 via email

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