Use artifacts layout in place of bin dir - #51

Merged
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir
Nov 18, 2019
Merged

Use artifacts layout in place of bin dir#51
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir

Conversation

@jashook

Copy link
Copy Markdown
Contributor

No description provided.

@jashook

Copy link
Copy Markdown
ContributorAuthor

/cc @ViktorHofer@safern

Also I asked @sbomer and @AaronRobinsonMSFT for a review

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

LGTM

Comment threadsrc/coreclr/artifactslogs/Restore_Product_Windows_NT__x64__Debug.wrn Outdated
jkoritzinsky
jkoritzinsky previously requested changes Nov 14, 2019
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threaddocs/coreclr/building/freebsd-instructions.md Outdated
Comment threadsrc/coreclr/build-test.sh Outdated
Comment threadsrc/coreclr/build.cmd Outdated
Comment threadsrc/coreclr/build.sh Outdated
Comment threadsrc/coreclr/tests/bringup_runtest.sh Outdated
Comment threadsrc/coreclr/tests/runtest.sh Outdated
@sbomer

Copy link
Copy Markdown
Member

The rest looks ok to me.

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

:shipit:

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

Arcade uses log instead of logs for the logs directory under artifacts.

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

@safern

Copy link
Copy Markdown
Member

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

Based on the changes I think it will be in the root, and that’s how it should be to be consistent, one central location.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This will put the artifacts folder under src/coreclr/artifacts. This can be changed to runtime/artifacts Is this the consensus?

@jkoritzinsky

Copy link
Copy Markdown
Member

I think we should change it to output to the root artifacts folder so all of the outputs follow the Arcade convention for location.

@ViktorHofer

Copy link
Copy Markdown
Member

Yeah, the artifacts folder should under the repo root.

@trylek

Copy link
Copy Markdown
Member

The change is getting pretty big - would there be any chance to merge in some parts of it that are sufficiently stable already? I completely agree that additional side cleanups like deleting bit-rotten or obsolete files should come next to avoid further complicating this change that is already complex enough.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I believe this change should pass CI with the last commit. We will see. Note dotnet/jitutils#227 should fixes the formatting jobs.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I do not have further plans to change this if this change passes CI.

@jashook

Copy link
Copy Markdown
ContributorAuthor

libraries ci checkout failure seems weird. @ViktorHofer

@jashook

Copy link
Copy Markdown
ContributorAuthor

Also seems unrelated to the change.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This change finally passed CI. Merging. Will start documentation after this goes in.

@jashook
jashook merged this pull request into dotnet:masterNov 18, 2019
@jashook
jashook deleted the move_to_artifacts_dir branch November 18, 2019 00:41
@jashook

Copy link
Copy Markdown
ContributorAuthor

Looks like I merged in place of squashed. That is very annoying.

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

You just merged and the repo's not public yet. You can force push to correct it.

@jashook

Copy link
Copy Markdown
ContributorAuthor

Done, I hate re-writing history. Hope this doesn't happen again.

@stephentoub

Copy link
Copy Markdown
Member

@jashook, when you just rewrote history, I assume you only went back as far as your commits? I'm asking because I'm looking at
71c981b
and the commit title doesn't match what was actually merged:
8650ad3
and on that PR it shows you having done something to it:
image
Something looks off here.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This seems to have been added as part of the rebase I did which squashed the merge changes I had... :(

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

And it changed the commit title? Looks like characters were deleted. That in and of itself isn't a big deal.. I just want to make sure something else worse didn't get messed up.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I didn't notice that, I have no idea why that happened.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

Looking closer.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I cannot explain why, but that commit title is how git displayed in my local enlistment. History before the rebase one my machine was:

commit 825c0427bc47070196ef252e84504f55bbb30bc9 (HEAD)
Merge: 40c184cbbf2 278fec7508a
Author: Jarret Shook <jashoo@microsoft.com>
Date: Sun Nov 17 16:41:02 2019 -0800
Merge pull request #51 from jashook/move_to_artifacts_dir
Use artifacts layout in place of bin dir
commit 40c184cbbf28f853a686f688cee90367a8daa75b
Author: Tomáš Rylek <trylek@microsoft.com>
Date: Sun Nov 17 21:55:56 2019 +0100
Add convenience scripts coreclr.sh / cmd for local user workflow (#81)
commit 278fec7508a941b4d01f9e8471a9ca27fc7ec02d
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 12:19:46 2019 -0800
Hopefully last path to fix
commit 6842a4ad3aeefff8585d10d4a89f130b15c3bc43
Author: Stephen Toub <stoub@microsoft.com>
Date: Sun Nov 17 15:18:47 2019 -0500
Move and t.CoreSetup.sln (#82)
Paths were wrong, and it shouldn't be in the repo root.
commit 84ca4a30f6d91747edab865f7cb6eadb9acac6e0
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 10:08:07 2019 -0800
Fix extremely stupid typo.
commit c61c164bbd3e6fae3d8fba177c849b9203926a46
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:20:50 2019 -0800
more syntax changes
commit 7f546b837f1bd6967dc4f6a4c0c524ef29163457
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:17:58 2019 -0800
Fix yml syntax
commit 6e3ef4b118026f8d350ffdf58da7dfb058b8e0e7
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:16:21 2019 -0800
clean ci?
commit b7cd7451949659a2869eae69a7f0f7077909eb76
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 22:57:19 2019 -0800
Fix bin dir location on windows
commit 4111dea524cc07f6ca49b3fcbff0d90581e168d7
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:21:32 2019 -0800
Address pr feedback and fix build break
commit 39140b1629a9f363369edc421644c9075f7712d6
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:01:54 2019 -0800
Change helix submit to use correct testhost location
commit 2a01d0a19739c0855999430530eeb86849734bd0
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 19:30:21 2019 -0800
Mop up ci change
commit 5dd072bdb83b61f20e85405707fcb15b5696be5b
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 17:49:20 2019 -0800
...

@jashook

Copy link
Copy Markdown
ContributorAuthor

On first guess, it was too long, but Tomas' change looks like it has more characters..

@jashook

Copy link
Copy Markdown
ContributorAuthor

I really apologize, I can do another push to amend the commit. Although admittedly I really would prefer to not much much more. @stephentoub let me know.

@stephentoub

Copy link
Copy Markdown
Member

If the only problem is the title on that commit, just leave it.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

No diff on contents. The history (minus the commit title you pointed) out looks correct to me. Minus the silly bit where I am added to the author in the github history. The git log itself correctly does not have me as an author.

@karelzkarelz added this to the 5.0.0 milestone Aug 18, 2020
@ghostghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@jashook@sbomer@safern@jkoritzinsky@ViktorHofer@trylek@stephentoub@AaronRobinsonMSFT@karelz
, '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

Use artifacts layout in place of bin dir - #51

Merged
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir
Nov 18, 2019
Merged

Use artifacts layout in place of bin dir#51
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir

Conversation

@jashook

Copy link
Copy Markdown
Contributor

No description provided.

@jashook

Copy link
Copy Markdown
ContributorAuthor

/cc @ViktorHofer@safern

Also I asked @sbomer and @AaronRobinsonMSFT for a review

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

LGTM

Comment threadsrc/coreclr/artifactslogs/Restore_Product_Windows_NT__x64__Debug.wrn Outdated
jkoritzinsky
jkoritzinsky previously requested changes Nov 14, 2019
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threaddocs/coreclr/building/freebsd-instructions.md Outdated
Comment threadsrc/coreclr/build-test.sh Outdated
Comment threadsrc/coreclr/build.cmd Outdated
Comment threadsrc/coreclr/build.sh Outdated
Comment threadsrc/coreclr/tests/bringup_runtest.sh Outdated
Comment threadsrc/coreclr/tests/runtest.sh Outdated
@sbomer

Copy link
Copy Markdown
Member

The rest looks ok to me.

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

:shipit:

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

Arcade uses log instead of logs for the logs directory under artifacts.

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

@safern

Copy link
Copy Markdown
Member

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

Based on the changes I think it will be in the root, and that’s how it should be to be consistent, one central location.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This will put the artifacts folder under src/coreclr/artifacts. This can be changed to runtime/artifacts Is this the consensus?

@jkoritzinsky

Copy link
Copy Markdown
Member

I think we should change it to output to the root artifacts folder so all of the outputs follow the Arcade convention for location.

@ViktorHofer

Copy link
Copy Markdown
Member

Yeah, the artifacts folder should under the repo root.

@trylek

Copy link
Copy Markdown
Member

The change is getting pretty big - would there be any chance to merge in some parts of it that are sufficiently stable already? I completely agree that additional side cleanups like deleting bit-rotten or obsolete files should come next to avoid further complicating this change that is already complex enough.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I believe this change should pass CI with the last commit. We will see. Note dotnet/jitutils#227 should fixes the formatting jobs.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I do not have further plans to change this if this change passes CI.

@jashook

Copy link
Copy Markdown
ContributorAuthor

libraries ci checkout failure seems weird. @ViktorHofer

@jashook

Copy link
Copy Markdown
ContributorAuthor

Also seems unrelated to the change.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This change finally passed CI. Merging. Will start documentation after this goes in.

@jashook
jashook merged this pull request into dotnet:masterNov 18, 2019
@jashook
jashook deleted the move_to_artifacts_dir branch November 18, 2019 00:41
@jashook

Copy link
Copy Markdown
ContributorAuthor

Looks like I merged in place of squashed. That is very annoying.

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

You just merged and the repo's not public yet. You can force push to correct it.

@jashook

Copy link
Copy Markdown
ContributorAuthor

Done, I hate re-writing history. Hope this doesn't happen again.

@stephentoub

Copy link
Copy Markdown
Member

@jashook, when you just rewrote history, I assume you only went back as far as your commits? I'm asking because I'm looking at
71c981b
and the commit title doesn't match what was actually merged:
8650ad3
and on that PR it shows you having done something to it:
image
Something looks off here.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This seems to have been added as part of the rebase I did which squashed the merge changes I had... :(

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

And it changed the commit title? Looks like characters were deleted. That in and of itself isn't a big deal.. I just want to make sure something else worse didn't get messed up.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I didn't notice that, I have no idea why that happened.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

Looking closer.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I cannot explain why, but that commit title is how git displayed in my local enlistment. History before the rebase one my machine was:

commit 825c0427bc47070196ef252e84504f55bbb30bc9 (HEAD)
Merge: 40c184cbbf2 278fec7508a
Author: Jarret Shook <jashoo@microsoft.com>
Date: Sun Nov 17 16:41:02 2019 -0800
Merge pull request #51 from jashook/move_to_artifacts_dir
Use artifacts layout in place of bin dir
commit 40c184cbbf28f853a686f688cee90367a8daa75b
Author: Tomáš Rylek <trylek@microsoft.com>
Date: Sun Nov 17 21:55:56 2019 +0100
Add convenience scripts coreclr.sh / cmd for local user workflow (#81)
commit 278fec7508a941b4d01f9e8471a9ca27fc7ec02d
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 12:19:46 2019 -0800
Hopefully last path to fix
commit 6842a4ad3aeefff8585d10d4a89f130b15c3bc43
Author: Stephen Toub <stoub@microsoft.com>
Date: Sun Nov 17 15:18:47 2019 -0500
Move and t.CoreSetup.sln (#82)
Paths were wrong, and it shouldn't be in the repo root.
commit 84ca4a30f6d91747edab865f7cb6eadb9acac6e0
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 10:08:07 2019 -0800
Fix extremely stupid typo.
commit c61c164bbd3e6fae3d8fba177c849b9203926a46
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:20:50 2019 -0800
more syntax changes
commit 7f546b837f1bd6967dc4f6a4c0c524ef29163457
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:17:58 2019 -0800
Fix yml syntax
commit 6e3ef4b118026f8d350ffdf58da7dfb058b8e0e7
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:16:21 2019 -0800
clean ci?
commit b7cd7451949659a2869eae69a7f0f7077909eb76
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 22:57:19 2019 -0800
Fix bin dir location on windows
commit 4111dea524cc07f6ca49b3fcbff0d90581e168d7
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:21:32 2019 -0800
Address pr feedback and fix build break
commit 39140b1629a9f363369edc421644c9075f7712d6
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:01:54 2019 -0800
Change helix submit to use correct testhost location
commit 2a01d0a19739c0855999430530eeb86849734bd0
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 19:30:21 2019 -0800
Mop up ci change
commit 5dd072bdb83b61f20e85405707fcb15b5696be5b
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 17:49:20 2019 -0800
...

@jashook

Copy link
Copy Markdown
ContributorAuthor

On first guess, it was too long, but Tomas' change looks like it has more characters..

@jashook

Copy link
Copy Markdown
ContributorAuthor

I really apologize, I can do another push to amend the commit. Although admittedly I really would prefer to not much much more. @stephentoub let me know.

@stephentoub

Copy link
Copy Markdown
Member

If the only problem is the title on that commit, just leave it.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

No diff on contents. The history (minus the commit title you pointed) out looks correct to me. Minus the silly bit where I am added to the author in the github history. The git log itself correctly does not have me as an author.

@karelzkarelz added this to the 5.0.0 milestone Aug 18, 2020
@ghostghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@jashook@sbomer@safern@jkoritzinsky@ViktorHofer@trylek@stephentoub@AaronRobinsonMSFT@karelz
, '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

Use artifacts layout in place of bin dir - #51

Merged
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir
Nov 18, 2019
Merged

Use artifacts layout in place of bin dir#51
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir

Conversation

@jashook

Copy link
Copy Markdown
Contributor

No description provided.

@jashook

Copy link
Copy Markdown
ContributorAuthor

/cc @ViktorHofer@safern

Also I asked @sbomer and @AaronRobinsonMSFT for a review

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

LGTM

Comment threadsrc/coreclr/artifactslogs/Restore_Product_Windows_NT__x64__Debug.wrn Outdated
jkoritzinsky
jkoritzinsky previously requested changes Nov 14, 2019
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threaddocs/coreclr/building/freebsd-instructions.md Outdated
Comment threadsrc/coreclr/build-test.sh Outdated
Comment threadsrc/coreclr/build.cmd Outdated
Comment threadsrc/coreclr/build.sh Outdated
Comment threadsrc/coreclr/tests/bringup_runtest.sh Outdated
Comment threadsrc/coreclr/tests/runtest.sh Outdated
@sbomer

Copy link
Copy Markdown
Member

The rest looks ok to me.

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

:shipit:

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

Arcade uses log instead of logs for the logs directory under artifacts.

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

@safern

Copy link
Copy Markdown
Member

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

Based on the changes I think it will be in the root, and that’s how it should be to be consistent, one central location.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This will put the artifacts folder under src/coreclr/artifacts. This can be changed to runtime/artifacts Is this the consensus?

@jkoritzinsky

Copy link
Copy Markdown
Member

I think we should change it to output to the root artifacts folder so all of the outputs follow the Arcade convention for location.

@ViktorHofer

Copy link
Copy Markdown
Member

Yeah, the artifacts folder should under the repo root.

@trylek

Copy link
Copy Markdown
Member

The change is getting pretty big - would there be any chance to merge in some parts of it that are sufficiently stable already? I completely agree that additional side cleanups like deleting bit-rotten or obsolete files should come next to avoid further complicating this change that is already complex enough.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I believe this change should pass CI with the last commit. We will see. Note dotnet/jitutils#227 should fixes the formatting jobs.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I do not have further plans to change this if this change passes CI.

@jashook

Copy link
Copy Markdown
ContributorAuthor

libraries ci checkout failure seems weird. @ViktorHofer

@jashook

Copy link
Copy Markdown
ContributorAuthor

Also seems unrelated to the change.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This change finally passed CI. Merging. Will start documentation after this goes in.

@jashook
jashook merged this pull request into dotnet:masterNov 18, 2019
@jashook
jashook deleted the move_to_artifacts_dir branch November 18, 2019 00:41
@jashook

Copy link
Copy Markdown
ContributorAuthor

Looks like I merged in place of squashed. That is very annoying.

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

You just merged and the repo's not public yet. You can force push to correct it.

@jashook

Copy link
Copy Markdown
ContributorAuthor

Done, I hate re-writing history. Hope this doesn't happen again.

@stephentoub

Copy link
Copy Markdown
Member

@jashook, when you just rewrote history, I assume you only went back as far as your commits? I'm asking because I'm looking at
71c981b
and the commit title doesn't match what was actually merged:
8650ad3
and on that PR it shows you having done something to it:
image
Something looks off here.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This seems to have been added as part of the rebase I did which squashed the merge changes I had... :(

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

And it changed the commit title? Looks like characters were deleted. That in and of itself isn't a big deal.. I just want to make sure something else worse didn't get messed up.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I didn't notice that, I have no idea why that happened.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

Looking closer.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I cannot explain why, but that commit title is how git displayed in my local enlistment. History before the rebase one my machine was:

commit 825c0427bc47070196ef252e84504f55bbb30bc9 (HEAD)
Merge: 40c184cbbf2 278fec7508a
Author: Jarret Shook <jashoo@microsoft.com>
Date: Sun Nov 17 16:41:02 2019 -0800
Merge pull request #51 from jashook/move_to_artifacts_dir
Use artifacts layout in place of bin dir
commit 40c184cbbf28f853a686f688cee90367a8daa75b
Author: Tomáš Rylek <trylek@microsoft.com>
Date: Sun Nov 17 21:55:56 2019 +0100
Add convenience scripts coreclr.sh / cmd for local user workflow (#81)
commit 278fec7508a941b4d01f9e8471a9ca27fc7ec02d
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 12:19:46 2019 -0800
Hopefully last path to fix
commit 6842a4ad3aeefff8585d10d4a89f130b15c3bc43
Author: Stephen Toub <stoub@microsoft.com>
Date: Sun Nov 17 15:18:47 2019 -0500
Move and t.CoreSetup.sln (#82)
Paths were wrong, and it shouldn't be in the repo root.
commit 84ca4a30f6d91747edab865f7cb6eadb9acac6e0
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 10:08:07 2019 -0800
Fix extremely stupid typo.
commit c61c164bbd3e6fae3d8fba177c849b9203926a46
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:20:50 2019 -0800
more syntax changes
commit 7f546b837f1bd6967dc4f6a4c0c524ef29163457
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:17:58 2019 -0800
Fix yml syntax
commit 6e3ef4b118026f8d350ffdf58da7dfb058b8e0e7
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:16:21 2019 -0800
clean ci?
commit b7cd7451949659a2869eae69a7f0f7077909eb76
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 22:57:19 2019 -0800
Fix bin dir location on windows
commit 4111dea524cc07f6ca49b3fcbff0d90581e168d7
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:21:32 2019 -0800
Address pr feedback and fix build break
commit 39140b1629a9f363369edc421644c9075f7712d6
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:01:54 2019 -0800
Change helix submit to use correct testhost location
commit 2a01d0a19739c0855999430530eeb86849734bd0
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 19:30:21 2019 -0800
Mop up ci change
commit 5dd072bdb83b61f20e85405707fcb15b5696be5b
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 17:49:20 2019 -0800
...

@jashook

Copy link
Copy Markdown
ContributorAuthor

On first guess, it was too long, but Tomas' change looks like it has more characters..

@jashook

Copy link
Copy Markdown
ContributorAuthor

I really apologize, I can do another push to amend the commit. Although admittedly I really would prefer to not much much more. @stephentoub let me know.

@stephentoub

Copy link
Copy Markdown
Member

If the only problem is the title on that commit, just leave it.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

No diff on contents. The history (minus the commit title you pointed) out looks correct to me. Minus the silly bit where I am added to the author in the github history. The git log itself correctly does not have me as an author.

@karelzkarelz added this to the 5.0.0 milestone Aug 18, 2020
@ghostghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@jashook@sbomer@safern@jkoritzinsky@ViktorHofer@trylek@stephentoub@AaronRobinsonMSFT@karelz
, '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

Use artifacts layout in place of bin dir - #51

Merged
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir
Nov 18, 2019
Merged

Use artifacts layout in place of bin dir#51
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir

Conversation

@jashook

Copy link
Copy Markdown
Contributor

No description provided.

@jashook

Copy link
Copy Markdown
ContributorAuthor

/cc @ViktorHofer@safern

Also I asked @sbomer and @AaronRobinsonMSFT for a review

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

LGTM

Comment threadsrc/coreclr/artifactslogs/Restore_Product_Windows_NT__x64__Debug.wrn Outdated
jkoritzinsky
jkoritzinsky previously requested changes Nov 14, 2019
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threaddocs/coreclr/building/freebsd-instructions.md Outdated
Comment threadsrc/coreclr/build-test.sh Outdated
Comment threadsrc/coreclr/build.cmd Outdated
Comment threadsrc/coreclr/build.sh Outdated
Comment threadsrc/coreclr/tests/bringup_runtest.sh Outdated
Comment threadsrc/coreclr/tests/runtest.sh Outdated
@sbomer

Copy link
Copy Markdown
Member

The rest looks ok to me.

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

:shipit:

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

Arcade uses log instead of logs for the logs directory under artifacts.

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

@safern

Copy link
Copy Markdown
Member

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

Based on the changes I think it will be in the root, and that’s how it should be to be consistent, one central location.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This will put the artifacts folder under src/coreclr/artifacts. This can be changed to runtime/artifacts Is this the consensus?

@jkoritzinsky

Copy link
Copy Markdown
Member

I think we should change it to output to the root artifacts folder so all of the outputs follow the Arcade convention for location.

@ViktorHofer

Copy link
Copy Markdown
Member

Yeah, the artifacts folder should under the repo root.

@trylek

Copy link
Copy Markdown
Member

The change is getting pretty big - would there be any chance to merge in some parts of it that are sufficiently stable already? I completely agree that additional side cleanups like deleting bit-rotten or obsolete files should come next to avoid further complicating this change that is already complex enough.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I believe this change should pass CI with the last commit. We will see. Note dotnet/jitutils#227 should fixes the formatting jobs.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I do not have further plans to change this if this change passes CI.

@jashook

Copy link
Copy Markdown
ContributorAuthor

libraries ci checkout failure seems weird. @ViktorHofer

@jashook

Copy link
Copy Markdown
ContributorAuthor

Also seems unrelated to the change.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This change finally passed CI. Merging. Will start documentation after this goes in.

@jashook
jashook merged this pull request into dotnet:masterNov 18, 2019
@jashook
jashook deleted the move_to_artifacts_dir branch November 18, 2019 00:41
@jashook

Copy link
Copy Markdown
ContributorAuthor

Looks like I merged in place of squashed. That is very annoying.

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

You just merged and the repo's not public yet. You can force push to correct it.

@jashook

Copy link
Copy Markdown
ContributorAuthor

Done, I hate re-writing history. Hope this doesn't happen again.

@stephentoub

Copy link
Copy Markdown
Member

@jashook, when you just rewrote history, I assume you only went back as far as your commits? I'm asking because I'm looking at
71c981b
and the commit title doesn't match what was actually merged:
8650ad3
and on that PR it shows you having done something to it:
image
Something looks off here.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This seems to have been added as part of the rebase I did which squashed the merge changes I had... :(

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

And it changed the commit title? Looks like characters were deleted. That in and of itself isn't a big deal.. I just want to make sure something else worse didn't get messed up.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I didn't notice that, I have no idea why that happened.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

Looking closer.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I cannot explain why, but that commit title is how git displayed in my local enlistment. History before the rebase one my machine was:

commit 825c0427bc47070196ef252e84504f55bbb30bc9 (HEAD)
Merge: 40c184cbbf2 278fec7508a
Author: Jarret Shook <jashoo@microsoft.com>
Date: Sun Nov 17 16:41:02 2019 -0800
Merge pull request #51 from jashook/move_to_artifacts_dir
Use artifacts layout in place of bin dir
commit 40c184cbbf28f853a686f688cee90367a8daa75b
Author: Tomáš Rylek <trylek@microsoft.com>
Date: Sun Nov 17 21:55:56 2019 +0100
Add convenience scripts coreclr.sh / cmd for local user workflow (#81)
commit 278fec7508a941b4d01f9e8471a9ca27fc7ec02d
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 12:19:46 2019 -0800
Hopefully last path to fix
commit 6842a4ad3aeefff8585d10d4a89f130b15c3bc43
Author: Stephen Toub <stoub@microsoft.com>
Date: Sun Nov 17 15:18:47 2019 -0500
Move and t.CoreSetup.sln (#82)
Paths were wrong, and it shouldn't be in the repo root.
commit 84ca4a30f6d91747edab865f7cb6eadb9acac6e0
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 10:08:07 2019 -0800
Fix extremely stupid typo.
commit c61c164bbd3e6fae3d8fba177c849b9203926a46
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:20:50 2019 -0800
more syntax changes
commit 7f546b837f1bd6967dc4f6a4c0c524ef29163457
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:17:58 2019 -0800
Fix yml syntax
commit 6e3ef4b118026f8d350ffdf58da7dfb058b8e0e7
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:16:21 2019 -0800
clean ci?
commit b7cd7451949659a2869eae69a7f0f7077909eb76
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 22:57:19 2019 -0800
Fix bin dir location on windows
commit 4111dea524cc07f6ca49b3fcbff0d90581e168d7
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:21:32 2019 -0800
Address pr feedback and fix build break
commit 39140b1629a9f363369edc421644c9075f7712d6
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:01:54 2019 -0800
Change helix submit to use correct testhost location
commit 2a01d0a19739c0855999430530eeb86849734bd0
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 19:30:21 2019 -0800
Mop up ci change
commit 5dd072bdb83b61f20e85405707fcb15b5696be5b
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 17:49:20 2019 -0800
...

@jashook

Copy link
Copy Markdown
ContributorAuthor

On first guess, it was too long, but Tomas' change looks like it has more characters..

@jashook

Copy link
Copy Markdown
ContributorAuthor

I really apologize, I can do another push to amend the commit. Although admittedly I really would prefer to not much much more. @stephentoub let me know.

@stephentoub

Copy link
Copy Markdown
Member

If the only problem is the title on that commit, just leave it.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

No diff on contents. The history (minus the commit title you pointed) out looks correct to me. Minus the silly bit where I am added to the author in the github history. The git log itself correctly does not have me as an author.

@karelzkarelz added this to the 5.0.0 milestone Aug 18, 2020
@ghostghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@jashook@sbomer@safern@jkoritzinsky@ViktorHofer@trylek@stephentoub@AaronRobinsonMSFT@karelz
, '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

Use artifacts layout in place of bin dir - #51

Merged
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir
Nov 18, 2019
Merged

Use artifacts layout in place of bin dir#51
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir

Conversation

@jashook

Copy link
Copy Markdown
Contributor

No description provided.

@jashook

Copy link
Copy Markdown
ContributorAuthor

/cc @ViktorHofer@safern

Also I asked @sbomer and @AaronRobinsonMSFT for a review

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

LGTM

Comment threadsrc/coreclr/artifactslogs/Restore_Product_Windows_NT__x64__Debug.wrn Outdated
jkoritzinsky
jkoritzinsky previously requested changes Nov 14, 2019
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threaddocs/coreclr/building/freebsd-instructions.md Outdated
Comment threadsrc/coreclr/build-test.sh Outdated
Comment threadsrc/coreclr/build.cmd Outdated
Comment threadsrc/coreclr/build.sh Outdated
Comment threadsrc/coreclr/tests/bringup_runtest.sh Outdated
Comment threadsrc/coreclr/tests/runtest.sh Outdated
@sbomer

Copy link
Copy Markdown
Member

The rest looks ok to me.

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

:shipit:

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

Arcade uses log instead of logs for the logs directory under artifacts.

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

@safern

Copy link
Copy Markdown
Member

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

Based on the changes I think it will be in the root, and that’s how it should be to be consistent, one central location.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This will put the artifacts folder under src/coreclr/artifacts. This can be changed to runtime/artifacts Is this the consensus?

@jkoritzinsky

Copy link
Copy Markdown
Member

I think we should change it to output to the root artifacts folder so all of the outputs follow the Arcade convention for location.

@ViktorHofer

Copy link
Copy Markdown
Member

Yeah, the artifacts folder should under the repo root.

@trylek

Copy link
Copy Markdown
Member

The change is getting pretty big - would there be any chance to merge in some parts of it that are sufficiently stable already? I completely agree that additional side cleanups like deleting bit-rotten or obsolete files should come next to avoid further complicating this change that is already complex enough.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I believe this change should pass CI with the last commit. We will see. Note dotnet/jitutils#227 should fixes the formatting jobs.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I do not have further plans to change this if this change passes CI.

@jashook

Copy link
Copy Markdown
ContributorAuthor

libraries ci checkout failure seems weird. @ViktorHofer

@jashook

Copy link
Copy Markdown
ContributorAuthor

Also seems unrelated to the change.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This change finally passed CI. Merging. Will start documentation after this goes in.

@jashook
jashook merged this pull request into dotnet:masterNov 18, 2019
@jashook
jashook deleted the move_to_artifacts_dir branch November 18, 2019 00:41
@jashook

Copy link
Copy Markdown
ContributorAuthor

Looks like I merged in place of squashed. That is very annoying.

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

You just merged and the repo's not public yet. You can force push to correct it.

@jashook

Copy link
Copy Markdown
ContributorAuthor

Done, I hate re-writing history. Hope this doesn't happen again.

@stephentoub

Copy link
Copy Markdown
Member

@jashook, when you just rewrote history, I assume you only went back as far as your commits? I'm asking because I'm looking at
71c981b
and the commit title doesn't match what was actually merged:
8650ad3
and on that PR it shows you having done something to it:
image
Something looks off here.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This seems to have been added as part of the rebase I did which squashed the merge changes I had... :(

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

And it changed the commit title? Looks like characters were deleted. That in and of itself isn't a big deal.. I just want to make sure something else worse didn't get messed up.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I didn't notice that, I have no idea why that happened.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

Looking closer.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I cannot explain why, but that commit title is how git displayed in my local enlistment. History before the rebase one my machine was:

commit 825c0427bc47070196ef252e84504f55bbb30bc9 (HEAD)
Merge: 40c184cbbf2 278fec7508a
Author: Jarret Shook <jashoo@microsoft.com>
Date: Sun Nov 17 16:41:02 2019 -0800
Merge pull request #51 from jashook/move_to_artifacts_dir
Use artifacts layout in place of bin dir
commit 40c184cbbf28f853a686f688cee90367a8daa75b
Author: Tomáš Rylek <trylek@microsoft.com>
Date: Sun Nov 17 21:55:56 2019 +0100
Add convenience scripts coreclr.sh / cmd for local user workflow (#81)
commit 278fec7508a941b4d01f9e8471a9ca27fc7ec02d
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 12:19:46 2019 -0800
Hopefully last path to fix
commit 6842a4ad3aeefff8585d10d4a89f130b15c3bc43
Author: Stephen Toub <stoub@microsoft.com>
Date: Sun Nov 17 15:18:47 2019 -0500
Move and t.CoreSetup.sln (#82)
Paths were wrong, and it shouldn't be in the repo root.
commit 84ca4a30f6d91747edab865f7cb6eadb9acac6e0
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 10:08:07 2019 -0800
Fix extremely stupid typo.
commit c61c164bbd3e6fae3d8fba177c849b9203926a46
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:20:50 2019 -0800
more syntax changes
commit 7f546b837f1bd6967dc4f6a4c0c524ef29163457
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:17:58 2019 -0800
Fix yml syntax
commit 6e3ef4b118026f8d350ffdf58da7dfb058b8e0e7
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:16:21 2019 -0800
clean ci?
commit b7cd7451949659a2869eae69a7f0f7077909eb76
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 22:57:19 2019 -0800
Fix bin dir location on windows
commit 4111dea524cc07f6ca49b3fcbff0d90581e168d7
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:21:32 2019 -0800
Address pr feedback and fix build break
commit 39140b1629a9f363369edc421644c9075f7712d6
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:01:54 2019 -0800
Change helix submit to use correct testhost location
commit 2a01d0a19739c0855999430530eeb86849734bd0
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 19:30:21 2019 -0800
Mop up ci change
commit 5dd072bdb83b61f20e85405707fcb15b5696be5b
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 17:49:20 2019 -0800
...

@jashook

Copy link
Copy Markdown
ContributorAuthor

On first guess, it was too long, but Tomas' change looks like it has more characters..

@jashook

Copy link
Copy Markdown
ContributorAuthor

I really apologize, I can do another push to amend the commit. Although admittedly I really would prefer to not much much more. @stephentoub let me know.

@stephentoub

Copy link
Copy Markdown
Member

If the only problem is the title on that commit, just leave it.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

No diff on contents. The history (minus the commit title you pointed) out looks correct to me. Minus the silly bit where I am added to the author in the github history. The git log itself correctly does not have me as an author.

@karelzkarelz added this to the 5.0.0 milestone Aug 18, 2020
@ghostghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@jashook@sbomer@safern@jkoritzinsky@ViktorHofer@trylek@stephentoub@AaronRobinsonMSFT@karelz
, '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

Use artifacts layout in place of bin dir - #51

Merged
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir
Nov 18, 2019
Merged

Use artifacts layout in place of bin dir#51
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir

Conversation

@jashook

Copy link
Copy Markdown
Contributor

No description provided.

@jashook

Copy link
Copy Markdown
ContributorAuthor

/cc @ViktorHofer@safern

Also I asked @sbomer and @AaronRobinsonMSFT for a review

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

LGTM

Comment threadsrc/coreclr/artifactslogs/Restore_Product_Windows_NT__x64__Debug.wrn Outdated
jkoritzinsky
jkoritzinsky previously requested changes Nov 14, 2019
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threaddocs/coreclr/building/freebsd-instructions.md Outdated
Comment threadsrc/coreclr/build-test.sh Outdated
Comment threadsrc/coreclr/build.cmd Outdated
Comment threadsrc/coreclr/build.sh Outdated
Comment threadsrc/coreclr/tests/bringup_runtest.sh Outdated
Comment threadsrc/coreclr/tests/runtest.sh Outdated
@sbomer

Copy link
Copy Markdown
Member

The rest looks ok to me.

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

:shipit:

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

Arcade uses log instead of logs for the logs directory under artifacts.

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

@safern

Copy link
Copy Markdown
Member

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

Based on the changes I think it will be in the root, and that’s how it should be to be consistent, one central location.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This will put the artifacts folder under src/coreclr/artifacts. This can be changed to runtime/artifacts Is this the consensus?

@jkoritzinsky

Copy link
Copy Markdown
Member

I think we should change it to output to the root artifacts folder so all of the outputs follow the Arcade convention for location.

@ViktorHofer

Copy link
Copy Markdown
Member

Yeah, the artifacts folder should under the repo root.

@trylek

Copy link
Copy Markdown
Member

The change is getting pretty big - would there be any chance to merge in some parts of it that are sufficiently stable already? I completely agree that additional side cleanups like deleting bit-rotten or obsolete files should come next to avoid further complicating this change that is already complex enough.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I believe this change should pass CI with the last commit. We will see. Note dotnet/jitutils#227 should fixes the formatting jobs.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I do not have further plans to change this if this change passes CI.

@jashook

Copy link
Copy Markdown
ContributorAuthor

libraries ci checkout failure seems weird. @ViktorHofer

@jashook

Copy link
Copy Markdown
ContributorAuthor

Also seems unrelated to the change.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This change finally passed CI. Merging. Will start documentation after this goes in.

@jashook
jashook merged this pull request into dotnet:masterNov 18, 2019
@jashook
jashook deleted the move_to_artifacts_dir branch November 18, 2019 00:41
@jashook

Copy link
Copy Markdown
ContributorAuthor

Looks like I merged in place of squashed. That is very annoying.

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

You just merged and the repo's not public yet. You can force push to correct it.

@jashook

Copy link
Copy Markdown
ContributorAuthor

Done, I hate re-writing history. Hope this doesn't happen again.

@stephentoub

Copy link
Copy Markdown
Member

@jashook, when you just rewrote history, I assume you only went back as far as your commits? I'm asking because I'm looking at
71c981b
and the commit title doesn't match what was actually merged:
8650ad3
and on that PR it shows you having done something to it:
image
Something looks off here.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This seems to have been added as part of the rebase I did which squashed the merge changes I had... :(

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

And it changed the commit title? Looks like characters were deleted. That in and of itself isn't a big deal.. I just want to make sure something else worse didn't get messed up.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I didn't notice that, I have no idea why that happened.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

Looking closer.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I cannot explain why, but that commit title is how git displayed in my local enlistment. History before the rebase one my machine was:

commit 825c0427bc47070196ef252e84504f55bbb30bc9 (HEAD)
Merge: 40c184cbbf2 278fec7508a
Author: Jarret Shook <jashoo@microsoft.com>
Date: Sun Nov 17 16:41:02 2019 -0800
Merge pull request #51 from jashook/move_to_artifacts_dir
Use artifacts layout in place of bin dir
commit 40c184cbbf28f853a686f688cee90367a8daa75b
Author: Tomáš Rylek <trylek@microsoft.com>
Date: Sun Nov 17 21:55:56 2019 +0100
Add convenience scripts coreclr.sh / cmd for local user workflow (#81)
commit 278fec7508a941b4d01f9e8471a9ca27fc7ec02d
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 12:19:46 2019 -0800
Hopefully last path to fix
commit 6842a4ad3aeefff8585d10d4a89f130b15c3bc43
Author: Stephen Toub <stoub@microsoft.com>
Date: Sun Nov 17 15:18:47 2019 -0500
Move and t.CoreSetup.sln (#82)
Paths were wrong, and it shouldn't be in the repo root.
commit 84ca4a30f6d91747edab865f7cb6eadb9acac6e0
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 10:08:07 2019 -0800
Fix extremely stupid typo.
commit c61c164bbd3e6fae3d8fba177c849b9203926a46
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:20:50 2019 -0800
more syntax changes
commit 7f546b837f1bd6967dc4f6a4c0c524ef29163457
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:17:58 2019 -0800
Fix yml syntax
commit 6e3ef4b118026f8d350ffdf58da7dfb058b8e0e7
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:16:21 2019 -0800
clean ci?
commit b7cd7451949659a2869eae69a7f0f7077909eb76
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 22:57:19 2019 -0800
Fix bin dir location on windows
commit 4111dea524cc07f6ca49b3fcbff0d90581e168d7
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:21:32 2019 -0800
Address pr feedback and fix build break
commit 39140b1629a9f363369edc421644c9075f7712d6
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:01:54 2019 -0800
Change helix submit to use correct testhost location
commit 2a01d0a19739c0855999430530eeb86849734bd0
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 19:30:21 2019 -0800
Mop up ci change
commit 5dd072bdb83b61f20e85405707fcb15b5696be5b
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 17:49:20 2019 -0800
...

@jashook

Copy link
Copy Markdown
ContributorAuthor

On first guess, it was too long, but Tomas' change looks like it has more characters..

@jashook

Copy link
Copy Markdown
ContributorAuthor

I really apologize, I can do another push to amend the commit. Although admittedly I really would prefer to not much much more. @stephentoub let me know.

@stephentoub

Copy link
Copy Markdown
Member

If the only problem is the title on that commit, just leave it.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

No diff on contents. The history (minus the commit title you pointed) out looks correct to me. Minus the silly bit where I am added to the author in the github history. The git log itself correctly does not have me as an author.

@karelzkarelz added this to the 5.0.0 milestone Aug 18, 2020
@ghostghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@jashook@sbomer@safern@jkoritzinsky@ViktorHofer@trylek@stephentoub@AaronRobinsonMSFT@karelz
, '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

Use artifacts layout in place of bin dir - #51

Merged
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir
Nov 18, 2019
Merged

Use artifacts layout in place of bin dir#51
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir

Conversation

@jashook

Copy link
Copy Markdown
Contributor

No description provided.

@jashook

Copy link
Copy Markdown
ContributorAuthor

/cc @ViktorHofer@safern

Also I asked @sbomer and @AaronRobinsonMSFT for a review

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

LGTM

Comment threadsrc/coreclr/artifactslogs/Restore_Product_Windows_NT__x64__Debug.wrn Outdated
jkoritzinsky
jkoritzinsky previously requested changes Nov 14, 2019
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threaddocs/coreclr/building/freebsd-instructions.md Outdated
Comment threadsrc/coreclr/build-test.sh Outdated
Comment threadsrc/coreclr/build.cmd Outdated
Comment threadsrc/coreclr/build.sh Outdated
Comment threadsrc/coreclr/tests/bringup_runtest.sh Outdated
Comment threadsrc/coreclr/tests/runtest.sh Outdated
@sbomer

Copy link
Copy Markdown
Member

The rest looks ok to me.

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

:shipit:

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

Arcade uses log instead of logs for the logs directory under artifacts.

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

@safern

Copy link
Copy Markdown
Member

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

Based on the changes I think it will be in the root, and that’s how it should be to be consistent, one central location.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This will put the artifacts folder under src/coreclr/artifacts. This can be changed to runtime/artifacts Is this the consensus?

@jkoritzinsky

Copy link
Copy Markdown
Member

I think we should change it to output to the root artifacts folder so all of the outputs follow the Arcade convention for location.

@ViktorHofer

Copy link
Copy Markdown
Member

Yeah, the artifacts folder should under the repo root.

@trylek

Copy link
Copy Markdown
Member

The change is getting pretty big - would there be any chance to merge in some parts of it that are sufficiently stable already? I completely agree that additional side cleanups like deleting bit-rotten or obsolete files should come next to avoid further complicating this change that is already complex enough.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I believe this change should pass CI with the last commit. We will see. Note dotnet/jitutils#227 should fixes the formatting jobs.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I do not have further plans to change this if this change passes CI.

@jashook

Copy link
Copy Markdown
ContributorAuthor

libraries ci checkout failure seems weird. @ViktorHofer

@jashook

Copy link
Copy Markdown
ContributorAuthor

Also seems unrelated to the change.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This change finally passed CI. Merging. Will start documentation after this goes in.

@jashook
jashook merged this pull request into dotnet:masterNov 18, 2019
@jashook
jashook deleted the move_to_artifacts_dir branch November 18, 2019 00:41
@jashook

Copy link
Copy Markdown
ContributorAuthor

Looks like I merged in place of squashed. That is very annoying.

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

You just merged and the repo's not public yet. You can force push to correct it.

@jashook

Copy link
Copy Markdown
ContributorAuthor

Done, I hate re-writing history. Hope this doesn't happen again.

@stephentoub

Copy link
Copy Markdown
Member

@jashook, when you just rewrote history, I assume you only went back as far as your commits? I'm asking because I'm looking at
71c981b
and the commit title doesn't match what was actually merged:
8650ad3
and on that PR it shows you having done something to it:
image
Something looks off here.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This seems to have been added as part of the rebase I did which squashed the merge changes I had... :(

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

And it changed the commit title? Looks like characters were deleted. That in and of itself isn't a big deal.. I just want to make sure something else worse didn't get messed up.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I didn't notice that, I have no idea why that happened.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

Looking closer.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I cannot explain why, but that commit title is how git displayed in my local enlistment. History before the rebase one my machine was:

commit 825c0427bc47070196ef252e84504f55bbb30bc9 (HEAD)
Merge: 40c184cbbf2 278fec7508a
Author: Jarret Shook <jashoo@microsoft.com>
Date: Sun Nov 17 16:41:02 2019 -0800
Merge pull request #51 from jashook/move_to_artifacts_dir
Use artifacts layout in place of bin dir
commit 40c184cbbf28f853a686f688cee90367a8daa75b
Author: Tomáš Rylek <trylek@microsoft.com>
Date: Sun Nov 17 21:55:56 2019 +0100
Add convenience scripts coreclr.sh / cmd for local user workflow (#81)
commit 278fec7508a941b4d01f9e8471a9ca27fc7ec02d
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 12:19:46 2019 -0800
Hopefully last path to fix
commit 6842a4ad3aeefff8585d10d4a89f130b15c3bc43
Author: Stephen Toub <stoub@microsoft.com>
Date: Sun Nov 17 15:18:47 2019 -0500
Move and t.CoreSetup.sln (#82)
Paths were wrong, and it shouldn't be in the repo root.
commit 84ca4a30f6d91747edab865f7cb6eadb9acac6e0
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 10:08:07 2019 -0800
Fix extremely stupid typo.
commit c61c164bbd3e6fae3d8fba177c849b9203926a46
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:20:50 2019 -0800
more syntax changes
commit 7f546b837f1bd6967dc4f6a4c0c524ef29163457
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:17:58 2019 -0800
Fix yml syntax
commit 6e3ef4b118026f8d350ffdf58da7dfb058b8e0e7
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:16:21 2019 -0800
clean ci?
commit b7cd7451949659a2869eae69a7f0f7077909eb76
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 22:57:19 2019 -0800
Fix bin dir location on windows
commit 4111dea524cc07f6ca49b3fcbff0d90581e168d7
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:21:32 2019 -0800
Address pr feedback and fix build break
commit 39140b1629a9f363369edc421644c9075f7712d6
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:01:54 2019 -0800
Change helix submit to use correct testhost location
commit 2a01d0a19739c0855999430530eeb86849734bd0
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 19:30:21 2019 -0800
Mop up ci change
commit 5dd072bdb83b61f20e85405707fcb15b5696be5b
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 17:49:20 2019 -0800
...

@jashook

Copy link
Copy Markdown
ContributorAuthor

On first guess, it was too long, but Tomas' change looks like it has more characters..

@jashook

Copy link
Copy Markdown
ContributorAuthor

I really apologize, I can do another push to amend the commit. Although admittedly I really would prefer to not much much more. @stephentoub let me know.

@stephentoub

Copy link
Copy Markdown
Member

If the only problem is the title on that commit, just leave it.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

No diff on contents. The history (minus the commit title you pointed) out looks correct to me. Minus the silly bit where I am added to the author in the github history. The git log itself correctly does not have me as an author.

@karelzkarelz added this to the 5.0.0 milestone Aug 18, 2020
@ghostghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@jashook@sbomer@safern@jkoritzinsky@ViktorHofer@trylek@stephentoub@AaronRobinsonMSFT@karelz
, '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

Use artifacts layout in place of bin dir - #51

Merged
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir
Nov 18, 2019
Merged

Use artifacts layout in place of bin dir#51
jashook merged 28 commits into
dotnet:masterfrom
jashook:move_to_artifacts_dir

Conversation

@jashook

Copy link
Copy Markdown
Contributor

No description provided.

@jashook

Copy link
Copy Markdown
ContributorAuthor

/cc @ViktorHofer@safern

Also I asked @sbomer and @AaronRobinsonMSFT for a review

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

LGTM

Comment threadsrc/coreclr/artifactslogs/Restore_Product_Windows_NT__x64__Debug.wrn Outdated
jkoritzinsky
jkoritzinsky previously requested changes Nov 14, 2019
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threadsrc/coreclr/build-test.cmd Outdated
Comment threaddocs/coreclr/building/freebsd-instructions.md Outdated
Comment threadsrc/coreclr/build-test.sh Outdated
Comment threadsrc/coreclr/build.cmd Outdated
Comment threadsrc/coreclr/build.sh Outdated
Comment threadsrc/coreclr/tests/bringup_runtest.sh Outdated
Comment threadsrc/coreclr/tests/runtest.sh Outdated
@sbomer

Copy link
Copy Markdown
Member

The rest looks ok to me.

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

:shipit:

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

Arcade uses log instead of logs for the logs directory under artifacts.

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

@safern

Copy link
Copy Markdown
Member

Also, is this going to output to the artifacts directory in the root or an artifacts directory under src/coreclr?

Based on the changes I think it will be in the root, and that’s how it should be to be consistent, one central location.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This will put the artifacts folder under src/coreclr/artifacts. This can be changed to runtime/artifacts Is this the consensus?

@jkoritzinsky

Copy link
Copy Markdown
Member

I think we should change it to output to the root artifacts folder so all of the outputs follow the Arcade convention for location.

@ViktorHofer

Copy link
Copy Markdown
Member

Yeah, the artifacts folder should under the repo root.

@trylek

Copy link
Copy Markdown
Member

The change is getting pretty big - would there be any chance to merge in some parts of it that are sufficiently stable already? I completely agree that additional side cleanups like deleting bit-rotten or obsolete files should come next to avoid further complicating this change that is already complex enough.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I believe this change should pass CI with the last commit. We will see. Note dotnet/jitutils#227 should fixes the formatting jobs.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I do not have further plans to change this if this change passes CI.

@jashook

Copy link
Copy Markdown
ContributorAuthor

libraries ci checkout failure seems weird. @ViktorHofer

@jashook

Copy link
Copy Markdown
ContributorAuthor

Also seems unrelated to the change.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This change finally passed CI. Merging. Will start documentation after this goes in.

@jashook
jashook merged this pull request into dotnet:masterNov 18, 2019
@jashook
jashook deleted the move_to_artifacts_dir branch November 18, 2019 00:41
@jashook

Copy link
Copy Markdown
ContributorAuthor

Looks like I merged in place of squashed. That is very annoying.

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

You just merged and the repo's not public yet. You can force push to correct it.

@jashook

Copy link
Copy Markdown
ContributorAuthor

Done, I hate re-writing history. Hope this doesn't happen again.

@stephentoub

Copy link
Copy Markdown
Member

@jashook, when you just rewrote history, I assume you only went back as far as your commits? I'm asking because I'm looking at
71c981b
and the commit title doesn't match what was actually merged:
8650ad3
and on that PR it shows you having done something to it:
image
Something looks off here.

@jashook

Copy link
Copy Markdown
ContributorAuthor

This seems to have been added as part of the rebase I did which squashed the merge changes I had... :(

@stephentoub

stephentoub commented Nov 18, 2019

Copy link
Copy Markdown
Member

And it changed the commit title? Looks like characters were deleted. That in and of itself isn't a big deal.. I just want to make sure something else worse didn't get messed up.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I didn't notice that, I have no idea why that happened.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

Looking closer.

@jashook

Copy link
Copy Markdown
ContributorAuthor

I cannot explain why, but that commit title is how git displayed in my local enlistment. History before the rebase one my machine was:

commit 825c0427bc47070196ef252e84504f55bbb30bc9 (HEAD)
Merge: 40c184cbbf2 278fec7508a
Author: Jarret Shook <jashoo@microsoft.com>
Date: Sun Nov 17 16:41:02 2019 -0800
Merge pull request #51 from jashook/move_to_artifacts_dir
Use artifacts layout in place of bin dir
commit 40c184cbbf28f853a686f688cee90367a8daa75b
Author: Tomáš Rylek <trylek@microsoft.com>
Date: Sun Nov 17 21:55:56 2019 +0100
Add convenience scripts coreclr.sh / cmd for local user workflow (#81)
commit 278fec7508a941b4d01f9e8471a9ca27fc7ec02d
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 12:19:46 2019 -0800
Hopefully last path to fix
commit 6842a4ad3aeefff8585d10d4a89f130b15c3bc43
Author: Stephen Toub <stoub@microsoft.com>
Date: Sun Nov 17 15:18:47 2019 -0500
Move and t.CoreSetup.sln (#82)
Paths were wrong, and it shouldn't be in the repo root.
commit 84ca4a30f6d91747edab865f7cb6eadb9acac6e0
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 10:08:07 2019 -0800
Fix extremely stupid typo.
commit c61c164bbd3e6fae3d8fba177c849b9203926a46
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:20:50 2019 -0800
more syntax changes
commit 7f546b837f1bd6967dc4f6a4c0c524ef29163457
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:17:58 2019 -0800
Fix yml syntax
commit 6e3ef4b118026f8d350ffdf58da7dfb058b8e0e7
Author: jashook <jashoo@microsoft.com>
Date: Sun Nov 17 07:16:21 2019 -0800
clean ci?
commit b7cd7451949659a2869eae69a7f0f7077909eb76
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 22:57:19 2019 -0800
Fix bin dir location on windows
commit 4111dea524cc07f6ca49b3fcbff0d90581e168d7
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:21:32 2019 -0800
Address pr feedback and fix build break
commit 39140b1629a9f363369edc421644c9075f7712d6
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 21:01:54 2019 -0800
Change helix submit to use correct testhost location
commit 2a01d0a19739c0855999430530eeb86849734bd0
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 19:30:21 2019 -0800
Mop up ci change
commit 5dd072bdb83b61f20e85405707fcb15b5696be5b
Author: jashook <jashoo@microsoft.com>
Date: Sat Nov 16 17:49:20 2019 -0800
...

@jashook

Copy link
Copy Markdown
ContributorAuthor

On first guess, it was too long, but Tomas' change looks like it has more characters..

@jashook

Copy link
Copy Markdown
ContributorAuthor

I really apologize, I can do another push to amend the commit. Although admittedly I really would prefer to not much much more. @stephentoub let me know.

@stephentoub

Copy link
Copy Markdown
Member

If the only problem is the title on that commit, just leave it.

@jashook

jashook commented Nov 18, 2019

Copy link
Copy Markdown
ContributorAuthor

No diff on contents. The history (minus the commit title you pointed) out looks correct to me. Minus the silly bit where I am added to the author in the github history. The git log itself correctly does not have me as an author.

@karelzkarelz added this to the 5.0.0 milestone Aug 18, 2020
@ghostghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@jashook@sbomer@safern@jkoritzinsky@ViktorHofer@trylek@stephentoub@AaronRobinsonMSFT@karelz