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

46.recovery #131

Closed
wants to merge 99 commits into from
Closed

46.recovery #131

wants to merge 99 commits into from

Conversation

YufengXin
Copy link
Collaborator

@sajith I restructured the pce code a little bit in which I created a new "simulation" sub fold where I moved all the 'research/simulation' scripts/examples into. They are not used in sdx operation. This is a precursory to my next commit addressing protection TE optimization that would happen later.

I rebased it on the current main. I am somehow puzzled by GitHub saying it's '100 commits behind main'. Maybe I used the git command incorrectly.

Please review and make sure it doesn't affect anything running. Thanks.

@YufengXin YufengXin requested a review from sajith July 25, 2023 19:59
@YufengXin
Copy link
Collaborator Author

YufengXin commented Jul 25, 2023

@sajith never mind. I will resolve the problems first, then ask you to review again.
The related files are all for the research/simulation purpose.

@YufengXin
Copy link
Collaborator Author

I think I should wait after you tag the main to 2.1.0 as you planned.

@YufengXin
Copy link
Collaborator Author

close for now, wait and redo it after tag 2.1.0

@YufengXin YufengXin closed this Jul 25, 2023
@sajith
Copy link
Member

sajith commented Jul 25, 2023

@YufengXin I don't think you need to hold this off, because the new code does not alter existing PCE APIs. You should be able to do something like this:

$ git fetch origin
$ git merge origin/main -m "merge main"
$ pip install black isort tox
$ black .   # format code
$ isort .   # sort imports
$ tox       # run tests

If everything goes well, update this branch, and then re-open this PR:

$ git push origin 46.recovery

That should work, from my quick testing. Please let me know if you need anything else.

@YufengXin
Copy link
Collaborator Author

Sure. I'll work on it. thanks.

BTW, this instruction should go to that "DeveloperGuidance" doc. -:) I used to use "git rebase", I'll start to use git merge.....

@sajith
Copy link
Member

sajith commented Jul 25, 2023

When working with Git and GitHub, there are many non-obvious things to try when things do not go as expected. It is usually better to follow Git or GitHub documentation, than trying to document it ourselves.

GitHub's PR diffs (sometimes? always?) seem confusing when using git rebase. I do not know why. So I just do git merge master.

As for other things (code formatting and testing), yes, we should get that stuff documented!

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