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

Assorted fixes #436

Closed
wants to merge 8 commits into from
Closed

Assorted fixes #436

wants to merge 8 commits into from

Conversation

ghedo
Copy link
Contributor

@ghedo ghedo commented Oct 8, 2015

Just a bunch of little fixes I've been accumulating.

@ghedo
Copy link
Contributor Author

ghedo commented Oct 8, 2015

I've merged in the commit from #423 as well.

@levitte
Copy link
Member

levitte commented Oct 22, 2015

Is this meant as master only?

@ghedo
Copy link
Contributor Author

ghedo commented Oct 23, 2015

@levitte I fixed all the issues locally, but I can 't currently push to GitHub (again...). I'll do it later.

Is this meant as master only?

They are all pretty simple fixes, so backporting them shouldn't be difficult. I can open new PRs for 1.0.2 and 1.0.1 if needed.

@levitte
Copy link
Member

levitte commented Oct 23, 2015

Backporting is usually the matter of a cherry pick and perhaps one or two small conflicts to resolve. However, I usually want to know the intent of a pull request, hence my question.

@ghedo
Copy link
Contributor Author

ghedo commented Oct 23, 2015

Ah ok. While they are all pretty small issues, I think they'd be worth backporting to at least 1.0.2.

I was also looking at the bugs/ directory, does it have any use? I think it can be removed along the other useless code.

@ghedo
Copy link
Contributor Author

ghedo commented Oct 23, 2015

I also just pushed the fixes for your comments.

@ghedo
Copy link
Contributor Author

ghedo commented Oct 23, 2015

There's also crypto/threads/ which doesn't seem to be used at all.

@ghedo
Copy link
Contributor Author

ghedo commented Oct 23, 2015

So I went ahead and removed bugs/ and crypto/threads/ as well. I left that in a separate commit though, so it can be easily discarded if you want.

I also have another patch that removes the remaining FIPS code (~500 lines) but I don't know if you want to get rid of that as well.

@levitte
Copy link
Member

levitte commented Oct 23, 2015

Actually, the removal of bugs/ and crypto/thread/ is something that I'd like to keep in master only, so ideally it should be in a different pull request.

@ghedo
Copy link
Contributor Author

ghedo commented Oct 23, 2015

Ok, I moved that commit to #448.

@levitte
Copy link
Member

levitte commented Oct 23, 2015

+1 as it stands now

@richsalz
Copy link
Contributor

+1 from me too. you want to commit/push? let me know if you want me to do it. (i agree master and 1.0.2)

@levitte
Copy link
Member

levitte commented Oct 23, 2015

I'll do it... and if we're backporting to 1.0.2, there's no reason whatsoever not to backport to 1.0.1.

@levitte
Copy link
Member

levitte commented Oct 23, 2015

Pushed on master, 1.0.2 and 1.0.1

@levitte levitte closed this Oct 23, 2015
@ghedo ghedo deleted the fixes branch October 23, 2015 20:04
@ghedo
Copy link
Contributor Author

ghedo commented Oct 24, 2015

Since this has been merged, RT#4090, RT#4081 and RT#4068 can be closed now.

@ghedo ghedo mentioned this pull request Oct 24, 2015
mcr pushed a commit to mcr/openssl that referenced this pull request Jun 22, 2021
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.

None yet

3 participants