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

GitHub: get API url and server URL from env #498

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gbruer15
Copy link

I encountered difficulties trying to use CompatHelper.jl on an Enterprise server, but it turns out I just needed to set the API url and the hostname correctly.

This pull request updates auto_detect_ci_service to automatically handle non-GitHub.com GitHub servers.

I am not sure how to add tests for this.

@DilumAluthge I see you are probably the most active here. Can you take a look at this and advise on tests?

@DilumAluthge
Copy link
Member

So, if we add this functionality into CompatHelper itself, it'll be a little difficult for us to test it.

Instead, could you modify your CompatHelper.yml file?

For example, if you look at the recommended CompatHelper.yml file, part of it looks like this:

- name: "Run CompatHelper"
run: |
import CompatHelper
CompatHelper.main()
shell: julia --color=yes {0}

You could instead modify your CompatHelper.yml file to look like this:

      - name: "Run CompatHelper"
        run: |
          import CompatHelper
          my_ci_cfg = CompatHelper.GitHubActions(;
              username = "...",
              email = "...",
              api_hostname = "...",
              clone_hostname = "...",
          )
          CompatHelper.main(ENV, my_ci_cfg)
        shell: julia --color=yes {0}

Where you'd fill in the relevant values in the GitHubActions(; ...) constructor.

That way, you can specify the exact values you need, without us needed to modify the source code of CompatHelper.jl (and thus needing to figure out a way to test it).

@gbruer15
Copy link
Author

I've already fixed it in my file (see below). If you prefer, I could fix it in the CompatHelper.yml in the repo instead of in the code.

If it's hard to test, I think it's fine to not test it. Since it doesn't break existing tests, it can't be worse than the current failure behavior. But whether it's tested or not, it should be fixed or the documentation should be corrected.

What do you think? Should I remove this fix and update the documentation instead? Or update the template CompatHelper.yml?

      - name: "Run CompatHelper"
        run: |
          import CompatHelper
          ci = CompatHelper.GitHubActions(;
            api_hostname = ENV["GITHUB_API_URL"],
            clone_hostname = replace(ENV["GITHUB_SERVER_URL"], r"^(https?://)?" => ""),
          )
          CompatHelper.main(ENV, ci)

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

Successfully merging this pull request may close these issues.

2 participants