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

Needs fix #97

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

Needs fix #97

wants to merge 3 commits into from

Conversation

Nic-Nic
Copy link
Member

@Nic-Nic Nic-Nic commented Feb 3, 2024

Kept the changes implemented by the user @diegommcc in a separate branch (3 commits in total) and reset them from master branch. The reason behind this is that those changes broke the package and its installation, causing trouble to users and other developers that have OmnipathR as a dependency. Please, be so kind as to fix the bugs and make sure everything works as intended before merging to master.

@Nic-Nic Nic-Nic added the bug Something isn't working label Feb 3, 2024
@Nic-Nic Nic-Nic requested a review from deeenes February 3, 2024 14:08
@deeenes
Copy link
Member

deeenes commented Feb 4, 2024

Hi @Nic-Nic , Thanks for doing the revert! @diegommcc I've done a major refactoring on your code, it will be ready soon. Then I'll ask you to review my code whether it produces the correct result. Thanks so much for this, I'm glad that we finally have this in OmniPath.

@diegommcc
Copy link
Collaborator

Hi @Nic-Nic, sorry for the inconvenience that I have created, when I merged to master, everything seemed to work fine for me. @deeenes, thanks a lot for checking the code. Let me know when I can help with the code revision. I am happy to collaborate in Omnipath, I'll try to avoid this to happen again.

@Nic-Nic
Copy link
Member Author

Nic-Nic commented Feb 5, 2024

Hi @diegommcc, no worries, can happen to anyone :)
Sorry if the measure seemed too drastic, but I thought it was better to prioritize that things worked back again for the users and packages having OmnipathR as a dependency, and then we could spend any time needed to fix any potential bugs without worries.
Also, @deeenes thanks for the help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants