|
|
Created:
March 28, 2018, 3:09 p.m. by a.shephard Modified:
April 10, 2018, 1:32 p.m. Visibility:
Public. |
DescriptionIssue 6501 - Remove CI references from KittCore
Patch Set 1 #
Total comments: 1
Patch Set 2 : Issue 6501 - Remove CI references from KittCore project #Patch Set 3 : Issue 6501 - Remove CI references from KittCore #Patch Set 4 : Issue 6501 - Remove CI refs from KittCore #MessagesTotal messages: 11
Couple of edits to the Build Phases. Removed the Circle CI references and had the scripts point at the Core Build Number value instead.
https://codereview.adblockplus.org/29735574/diff/29735575/KittCore.xcodeproj/... File KittCore.xcodeproj/project.pbxproj (right): https://codereview.adblockplus.org/29735574/diff/29735575/KittCore.xcodeproj/... KittCore.xcodeproj/project.pbxproj:1628: shellScript = "./deploy/build-jsapi.sh .. xcode $CORE_BUILD_NUMBER"; The build number doesn't seem to go here. See build-jsapi.sh: HAS_CI=$3 # unset or the specific CI env var saying "it's running in CI"
Good call. I've removed the CORE_BUILD_NUMBER variable from the JSAPI Build Phase so that it's now building as intended. What do you think about removing all CI references from the project and starting again? Is it worth to do that, or simply just use this as the way to remove CircleCI refs?
On 2018/04/03 11:17:00, a.shephard wrote: > Good call. I've removed the CORE_BUILD_NUMBER variable from the JSAPI Build > Phase so that it's now building as intended. > > What do you think about removing all CI references from the project and starting > again? Is it worth to do that, or simply just use this as the way to remove > CircleCI refs? I think it would be better to remove all of the logic involving Circle CI. Each file containing Circle CI logic could be a separate issue or everything could be done all at once. Either way, after every change, the build process should be tested to verify no errors have been introduced.
On 2018/04/03 22:50:17, d108 wrote: > On 2018/04/03 11:17:00, a.shephard wrote: > > Good call. I've removed the CORE_BUILD_NUMBER variable from the JSAPI Build > > Phase so that it's now building as intended. > > > > What do you think about removing all CI references from the project and > starting > > again? Is it worth to do that, or simply just use this as the way to remove > > CircleCI refs? > > I think it would be better to remove all of the logic involving Circle CI. Each > file containing Circle CI logic could be a separate issue or everything could be > done all at once. Either way, after every change, the build process should be > tested to verify no errors have been introduced. Patch Set 2 LGTM - I agree with Daniel. Removing all the CCI logic for now and evaluate whats needed when we experiment with GitLab CI later in this process.
On 2018/04/04 20:40:39, dean wrote: > On 2018/04/03 22:50:17, d108 wrote: > > On 2018/04/03 11:17:00, a.shephard wrote: > > > Good call. I've removed the CORE_BUILD_NUMBER variable from the JSAPI Build > > > Phase so that it's now building as intended. > > > > > > What do you think about removing all CI references from the project and > > starting > > > again? Is it worth to do that, or simply just use this as the way to remove > > > CircleCI refs? > > > > I think it would be better to remove all of the logic involving Circle CI. > Each > > file containing Circle CI logic could be a separate issue or everything could > be > > done all at once. Either way, after every change, the build process should be > > tested to verify no errors have been introduced. > > Patch Set 2 LGTM - I agree with Daniel. Removing all the CCI logic for now and > evaluate whats needed when we experiment with GitLab CI later in this process. OK, well technically the Circle CI references are gone. The only thing that really remains that would be CI related is the CORE_BUILD_NUMBER. I'll remove the CORE_BUILD_NUMBER parameter from the Build Phase for patching the Plist, which will deactivate any CI attempts. The two scripts I'll leave behind and they're being utilised; one to update the package.json file and the other is responsible for the JS API class files.
Patchset 3. Removed the references from KittCore so that it doesn't think CI is present.
On 2018/04/05 11:48:36, a.shephard wrote: > Patchset 3. Removed the references from KittCore so that it doesn't think CI is > present. LGTM patch set 3
Patchset 4. I just missed off one of the CI values on the Build Phases. I've removed it now.
On 2018/04/06 10:10:55, a.shephard wrote: > Patchset 4. I just missed off one of the CI values on the Build Phases. I've > removed it now. LGTM patch set 4
On 2018/04/06 23:51:57, dean wrote: > On 2018/04/06 10:10:55, a.shephard wrote: > > Patchset 4. I just missed off one of the CI values on the Build Phases. I've > > removed it now. > > LGTM patch set 4 LGTM patch set 4 |