Skip to content
This repository was archived by the owner on Nov 6, 2023. It is now read-only.

Re-enable MIT.xml - #12002

Merged
J0WI merged 16 commits into
re-mitfrom
unknown repository
Jan 3, 2018
Merged

Re-enable MIT.xml#12002
J0WI merged 16 commits into
re-mitfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Aug 20, 2017

Copy link
Copy Markdown

No description provided.

@J0WI

J0WI commented Sep 21, 2017

Copy link
Copy Markdown
Contributor

I preferred #12652 because that was a bugfix-only and was up to date with our master.
You have some additional updates in your PR. It would be great if you could rebase your branch, I'll have a look on it then.

@J0WI

J0WI commented Sep 23, 2017

Copy link
Copy Markdown
Contributor

Well, it turns out that the rule is still disabled after #12652

@ghost

ghost commented Sep 23, 2017

Copy link
Copy Markdown
Author

@J0WI Done.

<target host="mit-amps.mit.edu" />
<target host="mit150.mit.edu" />
<target host="mitei.mit.edu" />
<target host="mitei-members.mit.edu" />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment for it


<target host="eecs.mit.edu" />
<target host="www.eecs.mit.edu" />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add energy.mit.edu

<target host="techtv.mit.edu" />
<target host="tll.mit.edu" />
<target host="wayf.mit.edu" />
<!--target host="webmail.mit.edu" /-->

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WFM, please readd it and remove it from the comments.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for ^, www and web

@J0WI J0WI Oct 1, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like www has still some issues, but it can be redirected to web. Please have a look at the duplicates in MIT-mismatches.xml

@ghost ghost mentioned this pull request Oct 27, 2017

@wesinator wesinator left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add test for calendars to pass rules test

@jeremyn

jeremyn commented Dec 19, 2017

Copy link
Copy Markdown
Contributor

@J0WI I'd like to re-enable the MIT ruleset. What do you think we should do? We can't work with this PR because it's from @ghost now.

I ran fetch-test.sh against the current MIT.xml and only see two errors:

ERROR rules/MIT.xml: Fetch error: http://oeit-tsa.mit.edu/ => https://oeit-tsa.mit.edu/: (28, 'Connection timed out after 20001 milliseconds')
ERROR rules/MIT.xml: Fetch error: http://mitei-members.mit.edu/ => https://mitei-members.mit.edu/: (51, "SSL: no alternative certificate subject name matches target host name 'mitei-members.mit.edu'")

Should I just make another PR that just fixes these minimal problems? I don't want to do an exhaustive check for this huge site.

Also, what do you think about moving the keyserver https://pgp.mit.edu to its own ruleset just for that one thing? It's extra high value and I don't think it should depend on a hundred other MIT domains.

@J0WI

J0WI commented Dec 22, 2017

Copy link
Copy Markdown
Contributor

There is a lot of work here I would prefer to keep.
I can merge this in a new branch to keep all the commits. Then you can open a PR to this branch. I don't want to merge this in the master branch as long as it breaks stuff.

@jeremyn

jeremyn commented Dec 22, 2017

Copy link
Copy Markdown
Contributor

Re-enabling a complicated ruleset really means the whole thing needs to be reviewed, so my suggestion to just fix two things and then re-enable it is not a good approach.

I'm not really interested in reviewing an old, complicated, massive ruleset like this. In other words if you want to resubmit it as your own PR or something, go ahead, but it will probably sit there for a long time.

I would like to move pgp to its own ruleset since that is quick to do, and pgp is especially important to support.

@J0WI

J0WI commented Dec 22, 2017

Copy link
Copy Markdown
Contributor

I already reviewed parts of this PR, so I'll continue to review your changes until this is ready to merge.

@jeremyn

jeremyn commented Dec 22, 2017

Copy link
Copy Markdown
Contributor

I don't think we should merge commits from people who have deleted their accounts. It's like I wrote in 9f9c465 in #13887 :

  • We want to avoid including updates from contributors who have withdrawn permission for us to use their work. If a contributor deletes their GitHub account, it's not clear whether they have withdrawn their permission.

So in that case one of us would need to rewrite the old commits under their own name, and the other would have to review it. Either way I personally would need to review all the changes and I don't really feel like doing that.

@J0WI
J0WI changed the base branch from master to re-mit January 3, 2018 14:22
@J0WI
J0WI merged commit db12847 into EFForg:re-mit Jan 3, 2018
J0WI added a commit that referenced this pull request Jan 4, 2018
* Re-enable MIT.xml (#12002)

* Re-run Travis

* Update MIT.xml

* Update MIT.xml

* Update MIT.xml

* Update MIT.xml

* Update MIT.xml

* Update MIT-mismatches.xml

* Update MIT.xml

* Update MIT.xml

* Update MIT-mismatches.xml

* Update MIT.xml

* Update MIT.xml

* Update MIT.xml

* Update MIT.xml

* Update MIT.xml

* Normalize securecookie rule
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants