-
-
Notifications
You must be signed in to change notification settings - Fork 278
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
Add MIPS ISA #290
Add MIPS ISA #290
Conversation
As with any open-source contribution, i'd recommend that you:
|
To be honest I am not so familiar with git such as split the PRs into atomic PRs. We did this project on a different file so after 1 year we needed to folk this project, copy the files into this and change some lines so it will be compatible with qt6 |
No worries, I can try to guide you through this as much as possible. We're in no rush - you should see this as a learning opportunity as well. any open source contribution (or closed source, if you're in a workplace) will require you to do the above to ensure code quality and that your peers understand what you've done. And you, as a programmer, will benefit in having other people review and question your on your design descisions such that you can improve the work at hand and your own skillset. |
I searched some thing on how can I split the PRs and I found out that they use terminal commands for linux. Can I do it on Windows or do I need to use linux terminal? |
My best advice would be to first familiarize yourself with the process of contributing to open source projects (e.g. https://opensource.guide/how-to-contribute/, https://docs.github.com/en/get-started/quickstart/contributing-to-projects ...) and how to use git and github - google is your friend here. Next up is deciding how you want to submit this large codechange, again you can find information about this online. When having to do the same i personally ask myself what the set of atomic changes and features are in the change that i try to make, write those out, and then submit each of those changes as a separate PR. Practically speaking, you'd do this by creating a new branch and then applying the file edits of the code changes that you decided on. This can then be submitted as a PR. |
Okay I will try |
I created a new branch on my repository. Can I upload the changes here or do I need to create new pull request? |
Also does my code need to pass all the github tests? |
You would need to create pull requests against this repository, just as you've done with the PR we're discussing in now.
Yes, you cannot introduce code which breaks Ripes, that wouldn't be so good for the hundreds of other people who rely on it 😉. |
So I need to create new branch, then to pull request my changes to master branch. Then the code must past all the github check. Next I need to pull request the changes here and you will check the changes |
96d7ee3
to
5f71819
Compare
Now I removed all the changes so I can push one by one |
I have ready the PRs to upload them but some checks failed and I dont know how to find the problem. On qt everything is working fine and with VSRTL too on mortbopet/VSRTL#59 |
As I mentioned on mortbopet/VSRTL#59 I have ready small, atomic, PRs which do not depend on the changes in VSRTL. The only problem now is the tests because the tests are for RISCV ISA. Is it okay to upload the PRs to check them without these test for MIPS ISA? |
Good! Sounds like you have multiple PRs ready. Another suggestion is - don't try to do too much work up front before you start submitting these PRs. The whole point of the review process is that, first, you submit a small atomic change, and that change will be reviewed. The review process may highlight things which you need to change - and those changes may then break the PRs that you have planned after that. So, to avoid trying to boil the ocean here, i suggest that you submit one PR at a time, then we get that reviewed and checked in, and during that review process we'll determine whether or not a test should accompany that change. |
I have a lot of small PRs because some PRs are needed for the program to work correctly |
Add MIPS32 to isainfo.h
I submitted one PR to check it and tell me if you want it like this. If it is okay I will try to upload some othe small PRs to check them. Sorry for the mess with the PRs but I am still learning how to use GitHub |
No worries - are you referring to #308? |
Yes |
great - i think a fine cut point for a single PR that pertains to the ISA is to provide the whole ISA definition, e.g. the equivalent of: |
Add registers, syscall numbers and instructions opcodes and functions(for R-TYPE)
Like this? |
Yep! |
So now do I need to upload new commits with the things you told me to change or I can resolve it without new commits? |
You should:
Also see: |
I resolved some of your suggestions. For some others that I am not sure I commented below from your suggestions |
I think now is okay. Check it and let me know |
Looks good to me - merged! |
Perfect! So what is the next step? |
I mean which files do I need to upload |
Well, now you just continue the process that you did with this PR - i.e. look at your code and identify the next step/piece of code that is a candidate for an atomic PR, just like you did with this PR. If you're asking in practical terms, your PR is now merged into the master branch (c56dfb2) which means that you should rebase your existing work on top of the master branch - read: |
No description provided.