ARROW-12034: [Developer Tools] Formalize Minor PRs - #9763

Closed
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs
Closed

ARROW-12034: [Developer Tools] Formalize Minor PRs#9763
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs

Conversation

@emkornfield

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@emkornfieldemkornfield changed the title [Developer Tools] Formalize Minor PRsARROW-12034: [Developer Tools] Formalize Minor PRsMar 21, 2021
@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Is there a good way to test this, especially the GHA part?

@github-actions

Copy link
Copy Markdown

@kou

kou commented Mar 21, 2021

Copy link
Copy Markdown
Member

The PR title check job is ran on the master branch.

You can test this change by the following:

  1. Push the change to https://github.com/emkornfield/arrow/tree/master (not apache/arrow)
  2. Create a test PR on https://github.com/emkornfield/arrow (not apache/arrow)

e.g.: kou#10 is a test PR that was used to test my change.

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Thanks @kou, finally had a time to test this out. It seems to work. One question is if I should add a GHA that provides positive confirmation that the "MINOR:" title is correct.

@kou

kou commented Mar 28, 2021

Copy link
Copy Markdown
Member

I don't think that we need it but it may be able to implement by the following:

diff --git a/.github/workflows/dev_pr/link.js b/.github/workflows/dev_pr/link.js
index 550a9cd39..c27b5f8f1 100644
--- a/.github/workflows/dev_pr/link.js+++ b/.github/workflows/dev_pr/link.js@@ -19,6 +19,9 @@ function detectJIRAID(title) {
if (!title) {
return null;
}
+ if (title.startsWith("MINOR: ") {+ return "MINOR";+ }
const matched = /^(WIP:?\s*)?((ARROW|PARQUET)-\d+)/.exec(title);
if (!matched) {
return null;
@@ -47,15 +50,20 @@ async function haveComment(github, context, pullRequestNumber, body) {
}
async function commentJIRAURL(github, context, pullRequestNumber, jiraID) {
- const jiraURL = `https://issues.apache.org/jira/browse/${jiraID}`;- if (await haveComment(github, context, pullRequestNumber, jiraURL)) {+ let body;+ if (jiraID === "MINOR") {+ body = 'This is a minor PR.';+ } else {+ body = `https://issues.apache.org/jira/browse/${jiraID}`;+ }+ if (await haveComment(github, context, pullRequestNumber, body)) {
return;
}
await github.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pullRequestNumber,
- body: jiraURL+ body: body
});
}

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

@kou I tend to agree it might not be need, just might need some getting used of no GHA if MINOR is specified. If people complain, I'll try adding the proposed diff as a follow-up.

@alambalamb 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.

The basic idea looks good to me. Thank you @emkornfield

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@kou gave the pointer of testing on my fork above, which I did.

@wesm

wesm commented Mar 30, 2021

Copy link
Copy Markdown
Member

+1 from me on the content

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I'll wait until at least Friday and then merge unless anyone objects.

kou
kou approved these changes Mar 31, 2021

@koukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@emkornfield
emkornfield deleted the trivial_prs branch April 7, 2021 15:38
@asfimportasfimport mentioned this pull request Apr 4, 2021
Sign up for freeto 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.

4 participants

@emkornfield@kou@wesm@alamb
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

ARROW-12034: [Developer Tools] Formalize Minor PRs - #9763

Closed
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs
Closed

ARROW-12034: [Developer Tools] Formalize Minor PRs#9763
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs

Conversation

@emkornfield

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@emkornfieldemkornfield changed the title [Developer Tools] Formalize Minor PRsARROW-12034: [Developer Tools] Formalize Minor PRsMar 21, 2021
@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Is there a good way to test this, especially the GHA part?

@github-actions

Copy link
Copy Markdown

@kou

kou commented Mar 21, 2021

Copy link
Copy Markdown
Member

The PR title check job is ran on the master branch.

You can test this change by the following:

  1. Push the change to https://github.com/emkornfield/arrow/tree/master (not apache/arrow)
  2. Create a test PR on https://github.com/emkornfield/arrow (not apache/arrow)

e.g.: kou#10 is a test PR that was used to test my change.

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Thanks @kou, finally had a time to test this out. It seems to work. One question is if I should add a GHA that provides positive confirmation that the "MINOR:" title is correct.

@kou

kou commented Mar 28, 2021

Copy link
Copy Markdown
Member

I don't think that we need it but it may be able to implement by the following:

diff --git a/.github/workflows/dev_pr/link.js b/.github/workflows/dev_pr/link.js
index 550a9cd39..c27b5f8f1 100644
--- a/.github/workflows/dev_pr/link.js+++ b/.github/workflows/dev_pr/link.js@@ -19,6 +19,9 @@ function detectJIRAID(title) {
if (!title) {
return null;
}
+ if (title.startsWith("MINOR: ") {+ return "MINOR";+ }
const matched = /^(WIP:?\s*)?((ARROW|PARQUET)-\d+)/.exec(title);
if (!matched) {
return null;
@@ -47,15 +50,20 @@ async function haveComment(github, context, pullRequestNumber, body) {
}
async function commentJIRAURL(github, context, pullRequestNumber, jiraID) {
- const jiraURL = `https://issues.apache.org/jira/browse/${jiraID}`;- if (await haveComment(github, context, pullRequestNumber, jiraURL)) {+ let body;+ if (jiraID === "MINOR") {+ body = 'This is a minor PR.';+ } else {+ body = `https://issues.apache.org/jira/browse/${jiraID}`;+ }+ if (await haveComment(github, context, pullRequestNumber, body)) {
return;
}
await github.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pullRequestNumber,
- body: jiraURL+ body: body
});
}

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

@kou I tend to agree it might not be need, just might need some getting used of no GHA if MINOR is specified. If people complain, I'll try adding the proposed diff as a follow-up.

@alambalamb 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.

The basic idea looks good to me. Thank you @emkornfield

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@kou gave the pointer of testing on my fork above, which I did.

@wesm

wesm commented Mar 30, 2021

Copy link
Copy Markdown
Member

+1 from me on the content

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I'll wait until at least Friday and then merge unless anyone objects.

kou
kou approved these changes Mar 31, 2021

@koukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@emkornfield
emkornfield deleted the trivial_prs branch April 7, 2021 15:38
@asfimportasfimport mentioned this pull request Apr 4, 2021
Sign up for freeto 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.

4 participants

@emkornfield@kou@wesm@alamb
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

ARROW-12034: [Developer Tools] Formalize Minor PRs - #9763

Closed
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs
Closed

ARROW-12034: [Developer Tools] Formalize Minor PRs#9763
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs

Conversation

@emkornfield

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@emkornfieldemkornfield changed the title [Developer Tools] Formalize Minor PRsARROW-12034: [Developer Tools] Formalize Minor PRsMar 21, 2021
@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Is there a good way to test this, especially the GHA part?

@github-actions

Copy link
Copy Markdown

@kou

kou commented Mar 21, 2021

Copy link
Copy Markdown
Member

The PR title check job is ran on the master branch.

You can test this change by the following:

  1. Push the change to https://github.com/emkornfield/arrow/tree/master (not apache/arrow)
  2. Create a test PR on https://github.com/emkornfield/arrow (not apache/arrow)

e.g.: kou#10 is a test PR that was used to test my change.

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Thanks @kou, finally had a time to test this out. It seems to work. One question is if I should add a GHA that provides positive confirmation that the "MINOR:" title is correct.

@kou

kou commented Mar 28, 2021

Copy link
Copy Markdown
Member

I don't think that we need it but it may be able to implement by the following:

diff --git a/.github/workflows/dev_pr/link.js b/.github/workflows/dev_pr/link.js
index 550a9cd39..c27b5f8f1 100644
--- a/.github/workflows/dev_pr/link.js+++ b/.github/workflows/dev_pr/link.js@@ -19,6 +19,9 @@ function detectJIRAID(title) {
if (!title) {
return null;
}
+ if (title.startsWith("MINOR: ") {+ return "MINOR";+ }
const matched = /^(WIP:?\s*)?((ARROW|PARQUET)-\d+)/.exec(title);
if (!matched) {
return null;
@@ -47,15 +50,20 @@ async function haveComment(github, context, pullRequestNumber, body) {
}
async function commentJIRAURL(github, context, pullRequestNumber, jiraID) {
- const jiraURL = `https://issues.apache.org/jira/browse/${jiraID}`;- if (await haveComment(github, context, pullRequestNumber, jiraURL)) {+ let body;+ if (jiraID === "MINOR") {+ body = 'This is a minor PR.';+ } else {+ body = `https://issues.apache.org/jira/browse/${jiraID}`;+ }+ if (await haveComment(github, context, pullRequestNumber, body)) {
return;
}
await github.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pullRequestNumber,
- body: jiraURL+ body: body
});
}

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

@kou I tend to agree it might not be need, just might need some getting used of no GHA if MINOR is specified. If people complain, I'll try adding the proposed diff as a follow-up.

@alambalamb 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.

The basic idea looks good to me. Thank you @emkornfield

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@kou gave the pointer of testing on my fork above, which I did.

@wesm

wesm commented Mar 30, 2021

Copy link
Copy Markdown
Member

+1 from me on the content

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I'll wait until at least Friday and then merge unless anyone objects.

kou
kou approved these changes Mar 31, 2021

@koukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@emkornfield
emkornfield deleted the trivial_prs branch April 7, 2021 15:38
@asfimportasfimport mentioned this pull request Apr 4, 2021
Sign up for freeto 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.

4 participants

@emkornfield@kou@wesm@alamb
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

ARROW-12034: [Developer Tools] Formalize Minor PRs - #9763

Closed
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs
Closed

ARROW-12034: [Developer Tools] Formalize Minor PRs#9763
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs

Conversation

@emkornfield

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@emkornfieldemkornfield changed the title [Developer Tools] Formalize Minor PRsARROW-12034: [Developer Tools] Formalize Minor PRsMar 21, 2021
@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Is there a good way to test this, especially the GHA part?

@github-actions

Copy link
Copy Markdown

@kou

kou commented Mar 21, 2021

Copy link
Copy Markdown
Member

The PR title check job is ran on the master branch.

You can test this change by the following:

  1. Push the change to https://github.com/emkornfield/arrow/tree/master (not apache/arrow)
  2. Create a test PR on https://github.com/emkornfield/arrow (not apache/arrow)

e.g.: kou#10 is a test PR that was used to test my change.

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Thanks @kou, finally had a time to test this out. It seems to work. One question is if I should add a GHA that provides positive confirmation that the "MINOR:" title is correct.

@kou

kou commented Mar 28, 2021

Copy link
Copy Markdown
Member

I don't think that we need it but it may be able to implement by the following:

diff --git a/.github/workflows/dev_pr/link.js b/.github/workflows/dev_pr/link.js
index 550a9cd39..c27b5f8f1 100644
--- a/.github/workflows/dev_pr/link.js+++ b/.github/workflows/dev_pr/link.js@@ -19,6 +19,9 @@ function detectJIRAID(title) {
if (!title) {
return null;
}
+ if (title.startsWith("MINOR: ") {+ return "MINOR";+ }
const matched = /^(WIP:?\s*)?((ARROW|PARQUET)-\d+)/.exec(title);
if (!matched) {
return null;
@@ -47,15 +50,20 @@ async function haveComment(github, context, pullRequestNumber, body) {
}
async function commentJIRAURL(github, context, pullRequestNumber, jiraID) {
- const jiraURL = `https://issues.apache.org/jira/browse/${jiraID}`;- if (await haveComment(github, context, pullRequestNumber, jiraURL)) {+ let body;+ if (jiraID === "MINOR") {+ body = 'This is a minor PR.';+ } else {+ body = `https://issues.apache.org/jira/browse/${jiraID}`;+ }+ if (await haveComment(github, context, pullRequestNumber, body)) {
return;
}
await github.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pullRequestNumber,
- body: jiraURL+ body: body
});
}

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

@kou I tend to agree it might not be need, just might need some getting used of no GHA if MINOR is specified. If people complain, I'll try adding the proposed diff as a follow-up.

@alambalamb 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.

The basic idea looks good to me. Thank you @emkornfield

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@kou gave the pointer of testing on my fork above, which I did.

@wesm

wesm commented Mar 30, 2021

Copy link
Copy Markdown
Member

+1 from me on the content

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I'll wait until at least Friday and then merge unless anyone objects.

kou
kou approved these changes Mar 31, 2021

@koukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@emkornfield
emkornfield deleted the trivial_prs branch April 7, 2021 15:38
@asfimportasfimport mentioned this pull request Apr 4, 2021
Sign up for freeto 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.

4 participants

@emkornfield@kou@wesm@alamb
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

ARROW-12034: [Developer Tools] Formalize Minor PRs - #9763

Closed
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs
Closed

ARROW-12034: [Developer Tools] Formalize Minor PRs#9763
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs

Conversation

@emkornfield

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@emkornfieldemkornfield changed the title [Developer Tools] Formalize Minor PRsARROW-12034: [Developer Tools] Formalize Minor PRsMar 21, 2021
@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Is there a good way to test this, especially the GHA part?

@github-actions

Copy link
Copy Markdown

@kou

kou commented Mar 21, 2021

Copy link
Copy Markdown
Member

The PR title check job is ran on the master branch.

You can test this change by the following:

  1. Push the change to https://github.com/emkornfield/arrow/tree/master (not apache/arrow)
  2. Create a test PR on https://github.com/emkornfield/arrow (not apache/arrow)

e.g.: kou#10 is a test PR that was used to test my change.

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Thanks @kou, finally had a time to test this out. It seems to work. One question is if I should add a GHA that provides positive confirmation that the "MINOR:" title is correct.

@kou

kou commented Mar 28, 2021

Copy link
Copy Markdown
Member

I don't think that we need it but it may be able to implement by the following:

diff --git a/.github/workflows/dev_pr/link.js b/.github/workflows/dev_pr/link.js
index 550a9cd39..c27b5f8f1 100644
--- a/.github/workflows/dev_pr/link.js+++ b/.github/workflows/dev_pr/link.js@@ -19,6 +19,9 @@ function detectJIRAID(title) {
if (!title) {
return null;
}
+ if (title.startsWith("MINOR: ") {+ return "MINOR";+ }
const matched = /^(WIP:?\s*)?((ARROW|PARQUET)-\d+)/.exec(title);
if (!matched) {
return null;
@@ -47,15 +50,20 @@ async function haveComment(github, context, pullRequestNumber, body) {
}
async function commentJIRAURL(github, context, pullRequestNumber, jiraID) {
- const jiraURL = `https://issues.apache.org/jira/browse/${jiraID}`;- if (await haveComment(github, context, pullRequestNumber, jiraURL)) {+ let body;+ if (jiraID === "MINOR") {+ body = 'This is a minor PR.';+ } else {+ body = `https://issues.apache.org/jira/browse/${jiraID}`;+ }+ if (await haveComment(github, context, pullRequestNumber, body)) {
return;
}
await github.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pullRequestNumber,
- body: jiraURL+ body: body
});
}

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

@kou I tend to agree it might not be need, just might need some getting used of no GHA if MINOR is specified. If people complain, I'll try adding the proposed diff as a follow-up.

@alambalamb 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.

The basic idea looks good to me. Thank you @emkornfield

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@kou gave the pointer of testing on my fork above, which I did.

@wesm

wesm commented Mar 30, 2021

Copy link
Copy Markdown
Member

+1 from me on the content

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I'll wait until at least Friday and then merge unless anyone objects.

kou
kou approved these changes Mar 31, 2021

@koukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@emkornfield
emkornfield deleted the trivial_prs branch April 7, 2021 15:38
@asfimportasfimport mentioned this pull request Apr 4, 2021
Sign up for freeto 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.

4 participants

@emkornfield@kou@wesm@alamb
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

ARROW-12034: [Developer Tools] Formalize Minor PRs - #9763

Closed
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs
Closed

ARROW-12034: [Developer Tools] Formalize Minor PRs#9763
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs

Conversation

@emkornfield

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@emkornfieldemkornfield changed the title [Developer Tools] Formalize Minor PRsARROW-12034: [Developer Tools] Formalize Minor PRsMar 21, 2021
@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Is there a good way to test this, especially the GHA part?

@github-actions

Copy link
Copy Markdown

@kou

kou commented Mar 21, 2021

Copy link
Copy Markdown
Member

The PR title check job is ran on the master branch.

You can test this change by the following:

  1. Push the change to https://github.com/emkornfield/arrow/tree/master (not apache/arrow)
  2. Create a test PR on https://github.com/emkornfield/arrow (not apache/arrow)

e.g.: kou#10 is a test PR that was used to test my change.

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Thanks @kou, finally had a time to test this out. It seems to work. One question is if I should add a GHA that provides positive confirmation that the "MINOR:" title is correct.

@kou

kou commented Mar 28, 2021

Copy link
Copy Markdown
Member

I don't think that we need it but it may be able to implement by the following:

diff --git a/.github/workflows/dev_pr/link.js b/.github/workflows/dev_pr/link.js
index 550a9cd39..c27b5f8f1 100644
--- a/.github/workflows/dev_pr/link.js+++ b/.github/workflows/dev_pr/link.js@@ -19,6 +19,9 @@ function detectJIRAID(title) {
if (!title) {
return null;
}
+ if (title.startsWith("MINOR: ") {+ return "MINOR";+ }
const matched = /^(WIP:?\s*)?((ARROW|PARQUET)-\d+)/.exec(title);
if (!matched) {
return null;
@@ -47,15 +50,20 @@ async function haveComment(github, context, pullRequestNumber, body) {
}
async function commentJIRAURL(github, context, pullRequestNumber, jiraID) {
- const jiraURL = `https://issues.apache.org/jira/browse/${jiraID}`;- if (await haveComment(github, context, pullRequestNumber, jiraURL)) {+ let body;+ if (jiraID === "MINOR") {+ body = 'This is a minor PR.';+ } else {+ body = `https://issues.apache.org/jira/browse/${jiraID}`;+ }+ if (await haveComment(github, context, pullRequestNumber, body)) {
return;
}
await github.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pullRequestNumber,
- body: jiraURL+ body: body
});
}

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

@kou I tend to agree it might not be need, just might need some getting used of no GHA if MINOR is specified. If people complain, I'll try adding the proposed diff as a follow-up.

@alambalamb 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.

The basic idea looks good to me. Thank you @emkornfield

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@kou gave the pointer of testing on my fork above, which I did.

@wesm

wesm commented Mar 30, 2021

Copy link
Copy Markdown
Member

+1 from me on the content

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I'll wait until at least Friday and then merge unless anyone objects.

kou
kou approved these changes Mar 31, 2021

@koukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@emkornfield
emkornfield deleted the trivial_prs branch April 7, 2021 15:38
@asfimportasfimport mentioned this pull request Apr 4, 2021
Sign up for freeto 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.

4 participants

@emkornfield@kou@wesm@alamb
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

ARROW-12034: [Developer Tools] Formalize Minor PRs - #9763

Closed
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs
Closed

ARROW-12034: [Developer Tools] Formalize Minor PRs#9763
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs

Conversation

@emkornfield

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@emkornfieldemkornfield changed the title [Developer Tools] Formalize Minor PRsARROW-12034: [Developer Tools] Formalize Minor PRsMar 21, 2021
@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Is there a good way to test this, especially the GHA part?

@github-actions

Copy link
Copy Markdown

@kou

kou commented Mar 21, 2021

Copy link
Copy Markdown
Member

The PR title check job is ran on the master branch.

You can test this change by the following:

  1. Push the change to https://github.com/emkornfield/arrow/tree/master (not apache/arrow)
  2. Create a test PR on https://github.com/emkornfield/arrow (not apache/arrow)

e.g.: kou#10 is a test PR that was used to test my change.

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Thanks @kou, finally had a time to test this out. It seems to work. One question is if I should add a GHA that provides positive confirmation that the "MINOR:" title is correct.

@kou

kou commented Mar 28, 2021

Copy link
Copy Markdown
Member

I don't think that we need it but it may be able to implement by the following:

diff --git a/.github/workflows/dev_pr/link.js b/.github/workflows/dev_pr/link.js
index 550a9cd39..c27b5f8f1 100644
--- a/.github/workflows/dev_pr/link.js+++ b/.github/workflows/dev_pr/link.js@@ -19,6 +19,9 @@ function detectJIRAID(title) {
if (!title) {
return null;
}
+ if (title.startsWith("MINOR: ") {+ return "MINOR";+ }
const matched = /^(WIP:?\s*)?((ARROW|PARQUET)-\d+)/.exec(title);
if (!matched) {
return null;
@@ -47,15 +50,20 @@ async function haveComment(github, context, pullRequestNumber, body) {
}
async function commentJIRAURL(github, context, pullRequestNumber, jiraID) {
- const jiraURL = `https://issues.apache.org/jira/browse/${jiraID}`;- if (await haveComment(github, context, pullRequestNumber, jiraURL)) {+ let body;+ if (jiraID === "MINOR") {+ body = 'This is a minor PR.';+ } else {+ body = `https://issues.apache.org/jira/browse/${jiraID}`;+ }+ if (await haveComment(github, context, pullRequestNumber, body)) {
return;
}
await github.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pullRequestNumber,
- body: jiraURL+ body: body
});
}

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

@kou I tend to agree it might not be need, just might need some getting used of no GHA if MINOR is specified. If people complain, I'll try adding the proposed diff as a follow-up.

@alambalamb 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.

The basic idea looks good to me. Thank you @emkornfield

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@kou gave the pointer of testing on my fork above, which I did.

@wesm

wesm commented Mar 30, 2021

Copy link
Copy Markdown
Member

+1 from me on the content

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I'll wait until at least Friday and then merge unless anyone objects.

kou
kou approved these changes Mar 31, 2021

@koukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@emkornfield
emkornfield deleted the trivial_prs branch April 7, 2021 15:38
@asfimportasfimport mentioned this pull request Apr 4, 2021
Sign up for freeto 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.

4 participants

@emkornfield@kou@wesm@alamb
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

ARROW-12034: [Developer Tools] Formalize Minor PRs - #9763

Closed
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs
Closed

ARROW-12034: [Developer Tools] Formalize Minor PRs#9763
emkornfield wants to merge 3 commits into
apache:masterfrom
emkornfield:trivial_prs

Conversation

@emkornfield

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@emkornfieldemkornfield changed the title [Developer Tools] Formalize Minor PRsARROW-12034: [Developer Tools] Formalize Minor PRsMar 21, 2021
@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Is there a good way to test this, especially the GHA part?

@github-actions

Copy link
Copy Markdown

@kou

kou commented Mar 21, 2021

Copy link
Copy Markdown
Member

The PR title check job is ran on the master branch.

You can test this change by the following:

  1. Push the change to https://github.com/emkornfield/arrow/tree/master (not apache/arrow)
  2. Create a test PR on https://github.com/emkornfield/arrow (not apache/arrow)

e.g.: kou#10 is a test PR that was used to test my change.

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

Thanks @kou, finally had a time to test this out. It seems to work. One question is if I should add a GHA that provides positive confirmation that the "MINOR:" title is correct.

@kou

kou commented Mar 28, 2021

Copy link
Copy Markdown
Member

I don't think that we need it but it may be able to implement by the following:

diff --git a/.github/workflows/dev_pr/link.js b/.github/workflows/dev_pr/link.js
index 550a9cd39..c27b5f8f1 100644
--- a/.github/workflows/dev_pr/link.js+++ b/.github/workflows/dev_pr/link.js@@ -19,6 +19,9 @@ function detectJIRAID(title) {
if (!title) {
return null;
}
+ if (title.startsWith("MINOR: ") {+ return "MINOR";+ }
const matched = /^(WIP:?\s*)?((ARROW|PARQUET)-\d+)/.exec(title);
if (!matched) {
return null;
@@ -47,15 +50,20 @@ async function haveComment(github, context, pullRequestNumber, body) {
}
async function commentJIRAURL(github, context, pullRequestNumber, jiraID) {
- const jiraURL = `https://issues.apache.org/jira/browse/${jiraID}`;- if (await haveComment(github, context, pullRequestNumber, jiraURL)) {+ let body;+ if (jiraID === "MINOR") {+ body = 'This is a minor PR.';+ } else {+ body = `https://issues.apache.org/jira/browse/${jiraID}`;+ }+ if (await haveComment(github, context, pullRequestNumber, body)) {
return;
}
await github.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pullRequestNumber,
- body: jiraURL+ body: body
});
}

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

@kou I tend to agree it might not be need, just might need some getting used of no GHA if MINOR is specified. If people complain, I'll try adding the proposed diff as a follow-up.

@alambalamb 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.

The basic idea looks good to me. Thank you @emkornfield

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I do not know how to test the automation / scripts other than "in production" but that might be ok for this kind of PR

@kou gave the pointer of testing on my fork above, which I did.

@wesm

wesm commented Mar 30, 2021

Copy link
Copy Markdown
Member

+1 from me on the content

@emkornfield

Copy link
Copy Markdown
ContributorAuthor

I'll wait until at least Friday and then merge unless anyone objects.

kou
kou approved these changes Mar 31, 2021

@koukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@emkornfield
emkornfield deleted the trivial_prs branch April 7, 2021 15:38
@asfimportasfimport mentioned this pull request Apr 4, 2021
Sign up for freeto 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.

4 participants

@emkornfield@kou@wesm@alamb