Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add project sdk support for .net #787

Merged
merged 1 commit into from
Jan 24, 2020
Merged

Conversation

xt0rted
Copy link
Member

@xt0rted xt0rted commented Jan 24, 2020

Checklist:

  • If this PR is a new feature, please provide at least one example link
  • Make sure all of the significant new logic is covered by tests

This builds on #785 so the Sdk attribute is also handled now.

@stefanbuck
Copy link
Member

CI stucks , most likely caused by the force push. Maybe best to recreate the commit from scratch

@xt0rted
Copy link
Member Author

xt0rted commented Jan 24, 2020

It wasn't running before the force push. The workflow might need to be adjusted to something like this:

on:
  push:
    branches:
      - master
  pull_request:

That's how I set it up for GitHub Dark so it'd handle pushes to master, PRs from local branches, and PRs from forks.

Running on [push, pull_request] was causing extra runs when I'd create PRs so the branch filter was added to fix that.

@stefanbuck
Copy link
Member

A few minutes ago I remove the pull_request flag 8feff6f since I was annoyed by the extra run 😆 Thanks for sharing this template. Will update the workflow straight away

@stefanbuck
Copy link
Member

done

@stefanbuck stefanbuck merged commit 3781f93 into OctoLinker:master Jan 24, 2020
@xt0rted xt0rted deleted the project-sdk branch January 24, 2020 23:56
@xt0rted
Copy link
Member Author

xt0rted commented Jan 25, 2020

@stefanbuck I forgot the sdk value could contain multiple values like this Sdk="Microsoft.NET.Sdk;Microsoft.NET.Sdk.Publish". Is it possible to split these and create a link for each one?

@stefanbuck
Copy link
Member

That might be tricky. I need to look into this myself before I can say anything useful 😉

@stefanbuck
Copy link
Member

I looked into it and it's rather complicated to make this work. The current OctoLinker architecture is based on the assumption that each RegExp returns just one result. Changing this requires some larger rewrites down the line. Most likely not worth the effort.

@xt0rted
Copy link
Member Author

xt0rted commented Jan 26, 2020

From what I know having multiple values for this isn't common. I've never seen it done in a project outside of my own so I doubt many people would hit this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants