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

Unable to close milestone when running on AppVeyor but it works locally #439

Closed
Jericho opened this issue Mar 15, 2022 · 8 comments · Fixed by #576
Closed

Unable to close milestone when running on AppVeyor but it works locally #439

Jericho opened this issue Mar 15, 2022 · 8 comments · Fixed by #576
Assignees
Labels
Milestone

Comments

@Jericho
Copy link
Contributor

Jericho commented Mar 15, 2022

The last step of my Cake script when I publish a new release of my library is to close the GitHub milestone. Unfortunately, it's failing when my Cake script is running on AppVeyor (see AppVeyor log here and also screenshot provided below).

I'm running GitReleaseManager in Verbose mode but there is very little info about what went wrong which makes it tough to investigate the problem.

image

I executed the exact same command from my laptop and it completed successfully:
image

I have checked and double-checked the GitHub token configured in my AppVeyor account and I am confident that it is correct therefore I have ruled out that this could be caused by an authentication issue.

One thing I have noticed is that I am getting a warning about No valid version was found on Paging. In fact this warning is highlighted in red on AppVeyor which leads me to suspect that it could be more serious that a mere warning. I can see this same warning when I run GRM on my laptop but it's not highlighted, and GRM is able to continue to completion seemingly without any problems.

  • Is it possible that warnings are treated differently when running GRM on a CI versus locally?
  • Also, what does "no version found on paging" mean? Is this something I should investigate further or is it just an innocuous warning that I can safely ignore?
@Jericho
Copy link
Contributor Author

Jericho commented Mar 15, 2022

Ok, I figured out a few things:

  1. the warning about "no valid version" is reported by the MilestoneExtensions.Version extension method
  2. As I understand it, this extension method is invoked when GRM fetches all the milestones from my GitHub repo and it tries to infer a Version(major, minor, build) for each milestone with the assumption that the title contains a string that looks something like 'x.y.z'
  3. Paging which is mentioned in the warning refers to a milestone I created in my GitHub repo to conveniently group a bunch of issues that will be addressed in a future release. The name of this particular milestone does not follow the naming convention and therefore it's perfectly understandable for the extension method to warn that it was unable to derive a Version number from the title of this milestone.

So, either I am on a wild goose chase and this warning has nothing to do with the problem causing my build to fail or, as I mentioned in my original comment, this warning somehow gets treated as an exception but only on AppVeyor.

@gep13
Copy link
Member

gep13 commented Mar 15, 2022

@AdmiringWorm are you aware of any issues in this area? Specifically with regard to how things work on AppVeyor compared to locally?

@Jericho are you in a position to run a test without the Paging milestone in place, to see if the AppVeyor build succeeds?

@Jericho
Copy link
Contributor Author

Jericho commented Mar 15, 2022

are you in a position to run a test without the Paging milestone in place, to see if the AppVeyor build succeeds?

I renamed the "Paging" milestone to "99.99.99 - Paging" to satisfy the naming convention and launched a new build before going to lunch. Just came back and I see that the build completed successfully.

I think this proves that the warning is interpreted as a failure causing the build script to fail but only on AppVeyor.

@AdmiringWorm
Copy link
Member

@gep13 I am not aware of anything.
There is however one thing that could come into play, it is possible that both Warnings and Errors are outputted to stderr which maybe cause AppVeyor to abort the execution if something is written there.

@gep13
Copy link
Member

gep13 commented Mar 16, 2022

@AdmiringWorm is there a way to confirm what output is being used by GRM for warnings and errors? i.e. are we doing this wrong, and we should alter how these messages are being written out of the application?

@Jericho
Copy link
Contributor Author

Jericho commented Mar 16, 2022

While @AdmiringWorm is researching, does it make sense to change GRM to avoid invoking the Warning method and replace it with calls to either Verbose or Information?

There are 7 calls to Warning that would need to be modified and also 5 unit tests because they assert whether Warning was invoked or not.

I'm volunteering to submit a PR if you think it makes sense.

@AdmiringWorm
Copy link
Member

@gep13 we are specifically outputting warnings and higher to stderr. Anything lower will output to stdout.

There are divided opinions on the correct approach, and some believe only errors should be shown on stderr. In contrast, others believe both warnings and errors should be outputted there, as both are considered important information that should be outputted to the terminal even if the normal output is redirected.

I am hesitant to change it, though, but we may want to detect if we are running on appveyor and then output warnings to stdout instead (or if we are running on a CI in general).

Jericho added a commit to Jericho/ZoomNet that referenced this issue Aug 2, 2022
…ading coverage result to Coveralls.io or CodeCov. This change allows the build script to continue even when uploading coverage results fails.

This is the same problem I reported here: GitTools/GitReleaseManager#439
Jericho added a commit to Jericho/resources that referenced this issue Aug 3, 2022
…ading coverage result to Coveralls.io or CodeCov. This change allows the build script to continue even when uploading coverage results fails.

This is the same problem I reported here: GitTools/GitReleaseManager#439
@gep13 gep13 added this to the 0.14.0 milestone Aug 8, 2022
@gep13 gep13 removed this from the 0.14.0 milestone Nov 10, 2023
@AdmiringWorm AdmiringWorm self-assigned this Feb 8, 2024
AdmiringWorm added a commit to AdmiringWorm/GitReleaseManager that referenced this issue Feb 8, 2024
This updates the handling of how we output log messages to the console
to instead of blindly outputting warning messages to stderr all the
time, it will instead check if an environment variable called `CI` is
available and set to true, or the user themself have specified the
`--ci` argument.

When CI system is detected, any warnings will instead be outputted to
the normal stdout along with normal informational messages.
@gep13 gep13 added this to the 0.17.0 milestone Feb 12, 2024
gep13 added a commit that referenced this issue Feb 12, 2024
(#439) Add option to disable warning to stderr
gittools-bot pushed a commit that referenced this issue Feb 12, 2024
Merge pull request #576 from AdmiringWorm/AdmiringWorm/issue439

(#439) Add option to disable warning to stderr
gep13 added a commit that referenced this issue Mar 9, 2024
* release/0.17.0:
  (#578) Make the issue ID a long
  (GH-578) Bump Octokit to 10.0.0 to fix oversized ints
  (#574) Exclude all issues with label
  (#476) Add validation of input file path handler
  (#439) Add option to disable warning to stderr
@gittools-bot
Copy link

🎉 This issue has been resolved in version 0.17.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

gittools-bot pushed a commit that referenced this issue Mar 9, 2024
Merge branch 'release/0.17.0'

* release/0.17.0:
  (#578) Make the issue ID a long
  (GH-578) Bump Octokit to 10.0.0 to fix oversized ints
  (#574) Exclude all issues with label
  (#476) Add validation of input file path handler
  (#439) Add option to disable warning to stderr
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants