-
Notifications
You must be signed in to change notification settings - Fork 321
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
go118: Get VCS info from debug.BuildInfo #374
Conversation
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
//go:build !go1.18 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this future proof for >= 1.19?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like it is. I tested this by adding a test_go118.go
to an existing project:
//go:build go1.18
package main
import "log"
func init() {
log.Println("1.18")
}
Running this with Go 1.19 printed the line. Flipping it to //go:build !go1.18
made it go away when compiled/run with Go 1.19.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any chance this is going to get merged anytime soon? 🙂
if v.Value == "true" { | ||
modified = true | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to add vcs.modified
?
if modified { | ||
return rev + "-modified" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've seen this returned as a separate parameter, this is basically checking if git
was in a dirty state at the time of build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changing the API is out of scope here I'd say, as we see revision as one. it should still be pretty easy to parse as modified is not a valid git hash
Signed-off-by: Julien Pivotto <[email protected]>
42bc18d
to
5b6c049
Compare
@SuperQ addresses your comments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Signed-off-by: Julien Pivotto [email protected]