Skip to content

Conversation

@KOLANICH
Copy link

No description provided.

@rubo77
Copy link

rubo77 commented Sep 13, 2015

This seems like a complete rewrite in WSH (Windows Script Host)

Can you please add a lot more documentation in the code, so everyone can easily inderstand what is happening (without having to learn WSH too deep)?

Also the Comments that explained the reason, why to remove the updates are missing in your code.

A lot has to be enhanced before this should be published: The code should be easily understandable

@KOLANICH
Copy link
Author

Commented the code in doxygen short style.

@rubo77
Copy link

rubo77 commented Sep 14, 2015

Great work!

I didnt't try it out jet, but some more issues:

  • it seems your script is not outputing the Reason for each blacklisted KB like the current master version

  • the part annalyzing the string starting with "get" needs some more explanation

  • config.js needs comments that explain what to configure here
    Please add the Reason for each blacklisted KB maybe as a comment behind each KB number here like

     KB2902907, //< (Microsoft Security Essentials)
     KB2952664, //< (Get Windows 10 assistant)
     ...
    
  • Any dependencies?

  • libMSUpdater.js should reside in a subdirectory lib/ because it is not executable directly

Maybe WSH could be a vulnerability itself?

@KOLANICH
Copy link
Author

the part annalyzing the string starting with "get" needs some more explanation

It doesn't. It is everything clear. If WSH had at least ES5, everything would be implemented through setters/getters. But it doesn't.

config.js needs comments that explain what to configure here

config.js was meant to be config.json, but WHS js is built upon ancient ie engine and it doesn't have JSON. Adding a js-based parser is overkill here, that's why I use evil. But the config is still must be a valid json.

Any dependencies?

No dependencies except of WSH, ActiveX, used ActiveX components and my lib.

Maybe WSH could be a vulnerability itself?

We already have to use WSH, why not to use more WSH? WSH is a good technology, but strongly needs modernization. In fact i'd be better to replace it with node.js with bindings to .net and bundle node with windows.

@KOLANICH
Copy link
Author

Forget about this PR, I have made something better https://github.com/KOLANICH/CleanUnwantedUpdates

But I haven't managed to integrate it here, because of the problems with paths. When I run my script manually, it works, when I put it into bat, it doesn't. Maybe you can?

@rubo77
Copy link

rubo77 commented Sep 20, 2015

Some hints for working in github

There's only one rule: anything in the master branch is always deployable.

  • never work in the "master" baranch of a fork of another project.
  • create a branch in your forked repository and work in this branch so you can commit your pull request from there

In your new project, you should openly state the problem, that it is not working jet in the README, or others will download it and notice too late, that it is not working still

@KOLANICH
Copy link
Author

#25

@KOLANICH KOLANICH closed this Sep 20, 2015
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