search

chakra-core / ChakraCore

ChakraCore is an open source Javascript engine with a C API.
MIT License
9.06k stars 1.19k forks source link

To fix issue #6984, #include <stdarg.h> #6985

Closed EmployedRussian closed 2 months ago

EmployedRussian commented 2 months ago

The file uses va_arg etc, so it should #include

For #6984

rhuanjl commented 2 months ago

@EmployedRussian, sorry, added a "for" line to OP, that way Github would understand this is related.

The build "breakage" in CI is a check for the new copyright notice - we started doing that after we took over maintenance from Microsoft. The new notice is

// ChakraCore/Pal
// Contains portions (c) copyright Microsoft, portions copyright (c) the .NET Foundation and Contributors
// and edits (c) copyright the ChakraCore Contributors.
// See THIRD-PARTY-NOTICES.txt in the project root for .NET Foundation license
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.

It should replace these two lines:

// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

I can't remember if it was a conscious decision to only update copyright headers on files we've changed or if it was just the pattern we've slipped into - I wonder if we should run a script to update all copyright headers so this stops being a regular issue... If yes perhaps we could ignore the test failure and force merge this then I'll follow up with a generated PR to fix all the remaining copyright headers.

ppenzin commented 2 months ago

I can fix that with a bit of write permissions magic! :wink:

ppenzin commented 2 months ago

I added the correct license, but the tool didn't like it anyway. I think I will merge and open a PR to fix that

ppenzin commented 2 months ago

I was adding a line at the beginning only containing //, that is why the tool was not picking it up.

rhuanjl commented 2 months ago

Thanks for fixing this @EmployedRussian