search

simon987 / Much-Assembly-Required

Assembly programming game
https://muchassemblyrequired.com
GNU General Public License v3.0
925 stars 87 forks source link

Code style & refactor #197

Open simon987 opened 5 years ago

simon987 commented 5 years ago

Fix Issues in https://www.codefactor.io/repository/github/simon987/much-assembly-required/issues

Feel free to make a PR to fix any of the issues.

matias9477 commented 5 years ago

Hello, I wanna work in this, how can I help?

simon987 commented 5 years ago

@matias9477 You can visit the link above, there should be documentation on how to fix each issue

matias9477 commented 5 years ago

Hello again, I just made a pull request for a couple of minor fixes, this is my first time helping in a free software project. Thank you for being nice and I hope I can help a little more in the future.

simon987 commented 5 years ago

@matias9477 Thank you for you help and welcome to open source world :)

If you need any help don't hesitate to reach out (There is also a Slack channel)

cheeselover1 commented 5 years ago

Hi, @simon987. Like matias, I also made some similar changes to the code and submitted a PR. This is also my first time trying to contribute to open source project. Hope this could help you and I am really happy to help with the project in the future. BTW, the codefactor is fantastic, thanks for introducing such an amazing website for me as well. Thanks in advance

AstronautEVA commented 4 years ago

I am refactoring some things in editor.js and found some code I'm not understanding. Am I correct to say that && text.indexOf("o") === -1 && text.indexOf("0e") !== 0 located here is unnecessary?

I guess I just don't understand what they are testing for, but my thought is that any whole base-10 number or base-16 number is immediate, and that is tested in the first 2 conditions of !isNaN(Number(text)) && Number(text) === Math.floor(Number(text)).

simon987 commented 4 years ago

@jacquej96 Hi, Javascript allows octal ("o" prefix) and other kinds of numbers (I dont remember what "0e" is for) that the Java assembler does not understand, if we don't check for those, the code will produce an error when it is uploaded but the editor will not display it.

AstronautEVA commented 4 years ago

oh ok, good to know thanks!

AstronautEVA commented 4 years ago

Are there tests written for editor.js? Specifically the function getOperandType? I've refactored that function but cannot seem to find tests for it.

If there truly are no tests, should I write them and do a PR for that before submitting my PR for the refactor?

simon987 commented 4 years ago

@jacquej96 HI, There are no tests for client-side code. You can add the tests in the same PR (this is not necessary, since you would also need implement the initial test suite code)

adamrmelnyk commented 4 years ago

Hello, I've just started taking a look at this project and opened a small PR for some style issues. Let me know if there's something else I can add to this PR.

evanSpendlove commented 4 years ago

Hi, I'm new to open-source contributions, but would love to get involved with this issue. I have taken a look at the codefactor.io link and am working on opening a PR now.