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

Master: revert addition of broken config #3095

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Sigma1912
Copy link
Contributor

this reverts the changes made by these 4 commits:

dd88dc5
5c3aa31
5fea690
6e84469

and restores pyvcp_widgets.py to the last commit before these changes.

Reasons:

  1. most of these commits have no meaningful commit message
  2. adds two new folders to prominent places (ie root and configs directories) without following the established directory structure (eg using the authors username as folder name)
  3. contain absolute paths from the authors local machine
  4. author who pushed these has not replied to legitimate questions about these commits and the username seems unknown to GitHub, so it is unlikely that any messages are reaching the individual behind these commits.
  5. trailing white space in the code
  6. the config fails on load due to inappropriate absolute paths used

On a personal note I find it rather surprising that after 9 months and two new folders in prominent places this has apparently not attracted the attention of any of the other contributors with push rights to this project. If I were to file a PR like this I would be blown right out of the water and rightly so.

Thanks.

This reverts commit 5fea690.
…cpwidgets.py, so i added a space to make it dirty, then git add'ed it, then made this commit, then will check github again"

This reverts commit 5c3aa31.
@smoe
Copy link
Contributor

smoe commented Sep 1, 2024

I looked at the commits and am also a bit surprised. Was this a direct commit?

@hansu
Copy link
Member

hansu commented Sep 1, 2024

It looks like all are from #2756

I also agree that the "lib/grfx" looks weird and also the sim config should not be in a user's folder.

@Sigma1912
Copy link
Contributor Author

Note that 'grfx' is not in the 'lib' folder, it's a new root folder. I would also like to point out that there have been questions about this 8 months ago by another member:
5c3aa31

@hansu
Copy link
Member

hansu commented Sep 2, 2024

Note that 'grfx' is not in the 'lib' folder, it's a new root folder.

Oh...

Have you checked the functionality if we could benefit from that?

@andypugh
Copy link
Collaborator

andypugh commented Sep 2, 2024

Sorry, this is clearly my fault for not looking carefully at the commits. (or, more specifically, at where the files were going)
I was trying to tidy things up loose ends prior to the 2.9.2 release at the time, and obviously didn't give this enough attention.
Partly this was because it came from a long-term LinuxCNC contributor, Thomas Powderley

@Sigma1912
Copy link
Contributor Author

@hansu
I have not. Seems to add pyvcp widgets but I have not bothered to try and get the config working. I was actually hoping the original author would come back and fix it himself but after 9 months that is probably wishful thinking. I'm currently trying to clean up the sim configs and given the lack of feedback from most of the original authors I have decided to lobby for removal of the more complex one that fail on start up.
Actually there is also one I'd like to add but that is mine and well tested.

@andypugh
I wouldn't say this is your "fault", rather it seems to highlight an unfortunate situation of dwindling numbers of volunteers willing to carry a share of the workload.

@hansu
Copy link
Member

hansu commented Sep 2, 2024

The config is not working (even when adapting the paths like /home/tomp/linuxcnc-barwidgets/grfx/ ...) and there is no description provided for the config picker.

So I agree to revert those commits.

@hansu
Copy link
Member

hansu commented Sep 2, 2024

@tjtr33 Feel free to come with a new Pull Request when you cleaned this up a little bit and provide a working configuration (with description). Thanks!

@tjtr33
Copy link

tjtr33 commented Sep 2, 2024

Hello,
please remove/revert/obliveratee
all of my code/commits
whatever those are in GitSpeak
I have asked many time to remove my code
and was very surprised my last efforts
left content in the linuxcnc project
I cannot see the keyboard well enough to fix this
( macular degeneratttion)
Please make it all go away
nobody wants it anyway
TomP/tjtr33
Thx Andy fir trying to clean it up
but please just toss it all into the bit bucket

@hansu
Copy link
Member

hansu commented Sep 16, 2024

@tjtr33 Sad to hear that. But do you have some screenshots to see the functionality of your created widgets to see if they are worth doing some rework and integrating them?

@tjtr33
Copy link

tjtr33 commented Sep 19, 2024 via email

@hansu
Copy link
Member

hansu commented Sep 19, 2024

here is a screenshot

The screenshot seems missing...

@tjtr33
Copy link

tjtr33 commented Sep 20, 2024 via email

@Sigma1912
Copy link
Contributor Author

Still no screenshot, I think you may have to add the image directly in the github webpage.
#3095

@hansu
Copy link
Member

hansu commented Sep 20, 2024

You can simply drag the image into the text field. It will be uploaded automatically.

@tjtr33
Copy link

tjtr33 commented Sep 20, 2024

You can simply drag the image into the text field. It will be uploaded automatically.

pyvcp_bar_widgets-20240919

i only see text of file name and opening tyhe url gets me 404
pyvcp_bar_widgets-20240919

or goto imgr to avoid github nonsense
https://ibb.co/309NpsZ

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.

5 participants